| Message ID | 20240107181647.3049578-3-u@pkh.me |
|---|---|
| State | Accepted |
| Commit | 6d3591166786aa2404798331554472a125c96dc1 |
| Headers | show |
| Series | [FFmpeg-devel,1/5] avcodec/proresenc_anatoliy: use a compatible bitstream version | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
On date Sunday 2024-01-07 19:16:45 +0100, Clément Bœsch wrote: > This byte represents 4 reserved bits followed by 4 alpha_channel_type bits. > > alpha_channel_type currently has 3 differents defined values: 0 (no > alpha), 1 (8b alpha), and 2 (16b alpha), all the other values are > reserved. This part is correctly written (alpha_bits>>3 does the correct > thing), but the 4 initial bits are reserved. > --- > libavcodec/proresenc_kostya.c | 2 +- > tests/ref/vsynth/vsynth1-prores_ks | 2 +- > tests/ref/vsynth/vsynth2-prores_ks | 2 +- > tests/ref/vsynth/vsynth3-prores_ks | 2 +- > tests/ref/vsynth/vsynth_lena-prores_ks | 2 +- > 5 files changed, 5 insertions(+), 5 deletions(-) > > diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c > index de63127192..f6c71c2b86 100644 > --- a/libavcodec/proresenc_kostya.c > +++ b/libavcodec/proresenc_kostya.c > @@ -1051,7 +1051,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt, > bytestream_put_byte (&buf, pic->color_primaries); > bytestream_put_byte (&buf, pic->color_trc); > bytestream_put_byte (&buf, pic->colorspace); > - bytestream_put_byte (&buf, 0x40 | (ctx->alpha_bits >> 3)); > + bytestream_put_byte (&buf, ctx->alpha_bits >> 3); Shall be good, I wonder why it was done that way (probably there was no open specification at the time?).
On Mon, Jan 08, 2024 at 09:10:09PM +0100, Stefano Sabatini wrote: > On date Sunday 2024-01-07 19:16:45 +0100, Clément Bœsch wrote: > > This byte represents 4 reserved bits followed by 4 alpha_channel_type bits. > > > > alpha_channel_type currently has 3 differents defined values: 0 (no > > alpha), 1 (8b alpha), and 2 (16b alpha), all the other values are > > reserved. This part is correctly written (alpha_bits>>3 does the correct > > thing), but the 4 initial bits are reserved. > > --- > > libavcodec/proresenc_kostya.c | 2 +- > > tests/ref/vsynth/vsynth1-prores_ks | 2 +- > > tests/ref/vsynth/vsynth2-prores_ks | 2 +- > > tests/ref/vsynth/vsynth3-prores_ks | 2 +- > > tests/ref/vsynth/vsynth_lena-prores_ks | 2 +- > > 5 files changed, 5 insertions(+), 5 deletions(-) > > > > diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c > > index de63127192..f6c71c2b86 100644 > > --- a/libavcodec/proresenc_kostya.c > > +++ b/libavcodec/proresenc_kostya.c > > @@ -1051,7 +1051,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt, > > bytestream_put_byte (&buf, pic->color_primaries); > > bytestream_put_byte (&buf, pic->color_trc); > > bytestream_put_byte (&buf, pic->colorspace); > > - bytestream_put_byte (&buf, 0x40 | (ctx->alpha_bits >> 3)); > > + bytestream_put_byte (&buf, ctx->alpha_bits >> 3); > > Shall be good, I wonder why it was done that way (probably there was > no open specification at the time?). The Apple encoder tends to write stuff in this reserved area. Currently on the system I'm testing VideoToolBox is actually writing 0xE in the high nibble (instead of 0x4 here). So far it doesn't seem to have any impact (I tried to encode mov/prores files with and without alpha, with both ks and aw encoder, after this patchset, and the files play just fine with QuickTime). I have no idea what impact it has internally as I was unable to locate the encoder or the decoder in the Apple core frameworks.
diff --git a/libavcodec/proresenc_kostya.c b/libavcodec/proresenc_kostya.c index de63127192..f6c71c2b86 100644 --- a/libavcodec/proresenc_kostya.c +++ b/libavcodec/proresenc_kostya.c @@ -1051,7 +1051,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt, bytestream_put_byte (&buf, pic->color_primaries); bytestream_put_byte (&buf, pic->color_trc); bytestream_put_byte (&buf, pic->colorspace); - bytestream_put_byte (&buf, 0x40 | (ctx->alpha_bits >> 3)); + bytestream_put_byte (&buf, ctx->alpha_bits >> 3); bytestream_put_byte (&buf, 0); // reserved if (ctx->quant_sel != QUANT_MAT_DEFAULT) { bytestream_put_byte (&buf, 0x03); // matrix flags - both matrices are present diff --git a/tests/ref/vsynth/vsynth1-prores_ks b/tests/ref/vsynth/vsynth1-prores_ks index 22c248909c..a9aa6e41ba 100644 --- a/tests/ref/vsynth/vsynth1-prores_ks +++ b/tests/ref/vsynth/vsynth1-prores_ks @@ -1,4 +1,4 @@ -5b0970bacd4b03d70f7648fee2f0c85f *tests/data/fate/vsynth1-prores_ks.mov +fad50b4a0fb706fb2e282678ed962281 *tests/data/fate/vsynth1-prores_ks.mov 3858911 tests/data/fate/vsynth1-prores_ks.mov 100eb002413fe7a632d440dfbdf7e3ff *tests/data/fate/vsynth1-prores_ks.out.rawvideo stddev: 3.17 PSNR: 38.09 MAXDIFF: 39 bytes: 7603200/ 7603200 diff --git a/tests/ref/vsynth/vsynth2-prores_ks b/tests/ref/vsynth/vsynth2-prores_ks index 5186f55b4f..75289491ed 100644 --- a/tests/ref/vsynth/vsynth2-prores_ks +++ b/tests/ref/vsynth/vsynth2-prores_ks @@ -1,4 +1,4 @@ -abde4f84a5e4060492e3d8fcb56f2467 *tests/data/fate/vsynth2-prores_ks.mov +2f909bf4f1262da79dd2fa502cb41853 *tests/data/fate/vsynth2-prores_ks.mov 3868162 tests/data/fate/vsynth2-prores_ks.mov fe7ad707205c6100e9a3956d4e1c300e *tests/data/fate/vsynth2-prores_ks.out.rawvideo stddev: 1.17 PSNR: 46.72 MAXDIFF: 14 bytes: 7603200/ 7603200 diff --git a/tests/ref/vsynth/vsynth3-prores_ks b/tests/ref/vsynth/vsynth3-prores_ks index 561ee48dee..f859c9e5f8 100644 --- a/tests/ref/vsynth/vsynth3-prores_ks +++ b/tests/ref/vsynth/vsynth3-prores_ks @@ -1,4 +1,4 @@ -f6ce1e8e2272cea0592d3f969d48c1de *tests/data/fate/vsynth3-prores_ks.mov +3703ae6dea89c9d8b5a8872d8167ca42 *tests/data/fate/vsynth3-prores_ks.mov 95053 tests/data/fate/vsynth3-prores_ks.mov 9ab6d3e3cc7749796cd9fa984c60d890 *tests/data/fate/vsynth3-prores_ks.out.rawvideo stddev: 4.09 PSNR: 35.88 MAXDIFF: 35 bytes: 86700/ 86700 diff --git a/tests/ref/vsynth/vsynth_lena-prores_ks b/tests/ref/vsynth/vsynth_lena-prores_ks index 333578bc1e..c3f91de2c0 100644 --- a/tests/ref/vsynth/vsynth_lena-prores_ks +++ b/tests/ref/vsynth/vsynth_lena-prores_ks @@ -1,4 +1,4 @@ -86b9932d5f78d0b5836533e972a37a65 *tests/data/fate/vsynth_lena-prores_ks.mov +e0822c5ba6ce8825052dfb8dbf98d939 *tests/data/fate/vsynth_lena-prores_ks.mov 3884596 tests/data/fate/vsynth_lena-prores_ks.mov 6cfe987de99cf8ac9d43bdc5cd150838 *tests/data/fate/vsynth_lena-prores_ks.out.rawvideo stddev: 0.92 PSNR: 48.78 MAXDIFF: 10 bytes: 7603200/ 7603200