Vanilagy / mp4-muxer

MP4 multiplexer in pure TypeScript with support for WebCodecs API, video & audio.
https://vanilagy.github.io/mp4-muxer/demo
MIT License
419 stars 32 forks source link

TypeError: Cannot read properties of undefined (reading 'fullRange') #56

Closed lucadalli closed 4 months ago

lucadalli commented 4 months ago

Related to #55.

My decoderConfig derived from libavjs-webcodecs-bridge's videoStreamToConfig is missing colorSpace which results in an error at line: let thirdByte = (bitDepth << 4) + (chromaSubsampling << 1) + Number(decoderConfig.colorSpace.fullRange);

Vanilagy commented 4 months ago

Oh no

Vanilagy commented 4 months ago

Okay, I'm not familiar with that lib but it looks cool, so you need to brief me on this. When I use VideoEncoder with VP9, I get the colorSpace set in the decoder config correctly. Why is it that you aren't using this decoder config, but are retrieving it using the lib instead?

If the lib derives a decoder config that is supposed to mimic that created by browsers, it should probably also spit out a colorSpace object (that information is contained within the video stream).

I'm just trying to gauge if I should change my library to handle this case gracefully (in this case, what do I default to, though?), or if you're just passing the wrong thing.

lucadalli commented 4 months ago

I am demuxing a WebM using libav.js and muxing it to MP4 with mp4-muxer. I'm doing this so I can read it with MP4Box.js.

If there is no sane default and it cannot be omitted altogether, I think the most elegant way to solve this is to leave mp4-muxer as is and I'll need to derive the colorSpace object myself by using libav.js.

Just by glancing at the videoStreamToConfig do you have any idea how I can obtain colorSpace info?

lucadalli commented 4 months ago

Resolved by obtaining fullRange value via libav.js as follows:

const colorRange = await libav.AVCodecParameters_color_range(
  stream.codecpar,
)
const fullRange = colorRange === 2
const baseConfig = await videoStreamToConfig(libav, stream)
const config: VideoDecoderConfig = {
  ...baseConfig,
  colorSpace: {
    fullRange,
    ...(baseConfig.colorSpace || {}),
  },
}
Vanilagy commented 4 months ago

@lucadalli I guess your solution should suffice for now. I'm not sure if there is a sane default. There just are some fields that are required parts of the decoder config for some codecs. I'll try to keep those requirements in sync with what the WebCodecs API gives me.

In a future version, I might do my best to derive these values myself by also doing analysis on the actual video stream. It's funny since for VP9, it doesn't even matter that much, since decoders typically tend to use the config from the video stream anyway instead of from the headers in the container, as that's more reliable.

Vanilagy commented 4 months ago

I decided to leave the logic as-is but add a more meaningful and actionable error message instead of an internal TypeError.