search

ennuicastr / libavjs-webcodecs-polyfill

A polyfill for the WebCodecs API. No, really.
82 stars 8 forks source link

Fixmonostereo #17

Closed martenrichter closed 1 year ago

martenrichter commented 1 year ago

The first commit adds sample rate, extradata and the number of channels as information for codec allocation. As I went today for fixing the issues through all specifications, I have also included additional checks, if the configure format follows the spec: As currenlty only opus bitstream format is supported, it fails if ogg is requested as indicated by the description: https://www.w3.org/TR/webcodecs-opus-codec-registration/#audiodecoderconfig-description flac: https://www.w3.org/TR/webcodecs-flac-codec-registration/#audiodecoderconfig-description and vorbis https://www.w3.org/TR/webcodecs-vorbis-codec-registration/#audiodecoderconfig-description require extradata in every case. But I am unsure about the second commit, since it may break non-complying code, so I will revert it if you want, or you can cherrypick. An additional change in libav.js is required.

Yahweasel commented 1 year ago

Thanks! I don't have the time to review today, but it looks good. I'll probably import tomorrow or Tuesday.

martenrichter commented 1 year ago

Take your time, especially, if the extra data stuff works, I only had code to test opus. I am happy that it finally works, the media_type was the last missing piece.

Yahweasel commented 1 year ago

Oops, I forgot there was a PR in each ^^´

Yahweasel commented 1 year ago

One thing to address (above) before merge.

martenrichter commented 1 year ago

One thing to address (above) before merge.

Sorry, I do not see a comment from you above to consider, did you post something there or in the code for review? Or should I remove the checks?

Yahweasel commented 1 year ago

image

How does one GitHub? Why would this only be visible to me? GitHuuuuuuuub >: (

martenrichter commented 1 year ago

This is really weird, is it related to pending . May be because you started a review and it is only visible when finished? I think you can post there stuff without starting a review... Anyway I will look for the other thing.

martenrichter commented 1 year ago

Ok, the last commit should correct it. But it is untested in principle since my test setup uses only opus and this one never enters this code ( and I do not have a browser in the container). I assume you have tests for the formats flac and vorbis and can test if it makes a problem. And in principle, the same kind of changes but for codedwidth, displaywidth, and extradata (but only for avc) etc should also be required for the video decoder, but I do not have a working video setup (mostly because I do not think that performance would be great and my choice of codec, which prevents a software codec, yet).