Message ID | 20230319194240.15001-1-michael@niedermayer.cc |
---|---|
State | Accepted |
Commit | 006136efaf440d1876b06aae8a6403730c09069f |
Headers | show |
Series | [FFmpeg-devel,v2,1/3] avcodec/snowenc: AV_CODEC_CAP_ENCODER_RECON_FRAME support | expand |
On 3/19/2023 4:42 PM, Michael Niedermayer wrote: > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > --- > libavcodec/snowenc.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c > index 658684c575..5fb5906ed8 100644 > --- a/libavcodec/snowenc.c > +++ b/libavcodec/snowenc.c > @@ -26,6 +26,7 @@ > #include "avcodec.h" > #include "codec_internal.h" > #include "encode.h" > +#include "internal.h" //For AVCodecInternal.recon_frame > #include "me_cmp.h" > #include "packet_internal.h" > #include "snow_dwt.h" > @@ -1576,6 +1577,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt, > { > SnowContext *s = avctx->priv_data; > RangeCoder * const c= &s->c; > + AVCodecInternal *avci = avctx->internal; > AVFrame *pic; > const int width= s->avctx->width; > const int height= s->avctx->height; > @@ -1877,6 +1879,10 @@ redo_frame: > s->encoding_error, > (s->avctx->flags&AV_CODEC_FLAG_PSNR) ? SNOW_MAX_PLANES : 0, > s->current_picture->pict_type); > + if (s->avctx->flags & AV_CODEC_FLAG_RECON_FRAME) { > + av_frame_unref(avci->recon_frame); > + av_frame_ref(avci->recon_frame, s->current_picture); > + } > > pkt->size = ff_rac_terminate(c, 0); > if (s->current_picture->key_frame) > @@ -1934,7 +1940,9 @@ const FFCodec ff_snow_encoder = { > CODEC_LONG_NAME("Snow"), > .p.type = AVMEDIA_TYPE_VIDEO, > .p.id = AV_CODEC_ID_SNOW, > - .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE, > + .p.capabilities = AV_CODEC_CAP_DR1 | > + AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE | > + AV_CODEC_CAP_ENCODER_RECON_FRAME, > .priv_data_size = sizeof(SnowContext), > .init = encode_init, > FF_CODEC_ENCODE_CB(encode_frame), Testing with the tool from https://git.khirnov.net/libav.git/commit/?h=recon&id=d78e17840347abfe10b2bc7e789c60665de56e1f (Which i hope will be upstreamed eventually) i get $ tools/enc_recon_frame_test input.y4m snow flags=+bitexact 10 Checksum mismatch in frame ts=0, plane 0 I don't know if this really matters for snow (is it mean to be bitexact?), but it seems the very first frame returned by the reconstructed frame API is not the same as the one output by the snow decoder. The rest seem to match, though.
On Mon, Mar 20, 2023 at 07:07:59PM -0300, James Almer wrote: > On 3/19/2023 4:42 PM, Michael Niedermayer wrote: > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > --- > > libavcodec/snowenc.c | 10 +++++++++- > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c > > index 658684c575..5fb5906ed8 100644 > > --- a/libavcodec/snowenc.c > > +++ b/libavcodec/snowenc.c > > @@ -26,6 +26,7 @@ > > #include "avcodec.h" > > #include "codec_internal.h" > > #include "encode.h" > > +#include "internal.h" //For AVCodecInternal.recon_frame > > #include "me_cmp.h" > > #include "packet_internal.h" > > #include "snow_dwt.h" > > @@ -1576,6 +1577,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt, > > { > > SnowContext *s = avctx->priv_data; > > RangeCoder * const c= &s->c; > > + AVCodecInternal *avci = avctx->internal; > > AVFrame *pic; > > const int width= s->avctx->width; > > const int height= s->avctx->height; > > @@ -1877,6 +1879,10 @@ redo_frame: > > s->encoding_error, > > (s->avctx->flags&AV_CODEC_FLAG_PSNR) ? SNOW_MAX_PLANES : 0, > > s->current_picture->pict_type); > > + if (s->avctx->flags & AV_CODEC_FLAG_RECON_FRAME) { > > + av_frame_unref(avci->recon_frame); > > + av_frame_ref(avci->recon_frame, s->current_picture); > > + } > > pkt->size = ff_rac_terminate(c, 0); > > if (s->current_picture->key_frame) > > @@ -1934,7 +1940,9 @@ const FFCodec ff_snow_encoder = { > > CODEC_LONG_NAME("Snow"), > > .p.type = AVMEDIA_TYPE_VIDEO, > > .p.id = AV_CODEC_ID_SNOW, > > - .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE, > > + .p.capabilities = AV_CODEC_CAP_DR1 | > > + AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE | > > + AV_CODEC_CAP_ENCODER_RECON_FRAME, > > .priv_data_size = sizeof(SnowContext), > > .init = encode_init, > > FF_CODEC_ENCODE_CB(encode_frame), > > Testing with the tool from https://git.khirnov.net/libav.git/commit/?h=recon&id=d78e17840347abfe10b2bc7e789c60665de56e1f > (Which i hope will be upstreamed eventually) i get > > $ tools/enc_recon_frame_test input.y4m snow flags=+bitexact 10 > Checksum mismatch in frame ts=0, plane 0 I cannot replicate this, using matrixbench and above command line i get no mismatches, where can i find the input.y4m you used ? also would be nice if this tool would write a image showing the difference thx > > I don't know if this really matters for snow (is it mean to be bitexact?), > but it seems the very first frame returned by the reconstructed frame API is > not the same as the one output by the snow decoder. The rest seem to match, > though. This sounds rather strange, the first frame would be used as reference for motion compensation so the subsequent frames matching after a mismatch is "surprising" thx [...]
On 3/23/2023 6:50 AM, Michael Niedermayer wrote: > On Mon, Mar 20, 2023 at 07:07:59PM -0300, James Almer wrote: >> On 3/19/2023 4:42 PM, Michael Niedermayer wrote: >>> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> >>> --- >>> libavcodec/snowenc.c | 10 +++++++++- >>> 1 file changed, 9 insertions(+), 1 deletion(-) >>> >>> diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c >>> index 658684c575..5fb5906ed8 100644 >>> --- a/libavcodec/snowenc.c >>> +++ b/libavcodec/snowenc.c >>> @@ -26,6 +26,7 @@ >>> #include "avcodec.h" >>> #include "codec_internal.h" >>> #include "encode.h" >>> +#include "internal.h" //For AVCodecInternal.recon_frame >>> #include "me_cmp.h" >>> #include "packet_internal.h" >>> #include "snow_dwt.h" >>> @@ -1576,6 +1577,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt, >>> { >>> SnowContext *s = avctx->priv_data; >>> RangeCoder * const c= &s->c; >>> + AVCodecInternal *avci = avctx->internal; >>> AVFrame *pic; >>> const int width= s->avctx->width; >>> const int height= s->avctx->height; >>> @@ -1877,6 +1879,10 @@ redo_frame: >>> s->encoding_error, >>> (s->avctx->flags&AV_CODEC_FLAG_PSNR) ? SNOW_MAX_PLANES : 0, >>> s->current_picture->pict_type); >>> + if (s->avctx->flags & AV_CODEC_FLAG_RECON_FRAME) { >>> + av_frame_unref(avci->recon_frame); >>> + av_frame_ref(avci->recon_frame, s->current_picture); >>> + } >>> pkt->size = ff_rac_terminate(c, 0); >>> if (s->current_picture->key_frame) >>> @@ -1934,7 +1940,9 @@ const FFCodec ff_snow_encoder = { >>> CODEC_LONG_NAME("Snow"), >>> .p.type = AVMEDIA_TYPE_VIDEO, >>> .p.id = AV_CODEC_ID_SNOW, >>> - .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE, >>> + .p.capabilities = AV_CODEC_CAP_DR1 | >>> + AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE | >>> + AV_CODEC_CAP_ENCODER_RECON_FRAME, >>> .priv_data_size = sizeof(SnowContext), >>> .init = encode_init, >>> FF_CODEC_ENCODE_CB(encode_frame), >> >> Testing with the tool from https://git.khirnov.net/libav.git/commit/?h=recon&id=d78e17840347abfe10b2bc7e789c60665de56e1f >> (Which i hope will be upstreamed eventually) i get >> >> $ tools/enc_recon_frame_test input.y4m snow flags=+bitexact 10 >> Checksum mismatch in frame ts=0, plane 0 > > I cannot replicate this, using matrixbench and above command line i get no > mismatches, where can i find the input.y4m you used ? It was https://0x0.st/sfRh.y4m > > also would be nice if this tool would write a image showing the difference > > thx > >> >> I don't know if this really matters for snow (is it mean to be bitexact?), >> but it seems the very first frame returned by the reconstructed frame API is >> not the same as the one output by the snow decoder. The rest seem to match, >> though. > > This sounds rather strange, the first frame would be used as reference for > motion compensation so the subsequent frames matching after a mismatch is > "surprising" I tried other supported encoders like libx264 and librav1e and none of them report mismatches. > > thx > > [...] > > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Thu, Mar 23, 2023 at 08:32:48AM -0300, James Almer wrote: > On 3/23/2023 6:50 AM, Michael Niedermayer wrote: > > On Mon, Mar 20, 2023 at 07:07:59PM -0300, James Almer wrote: > > > On 3/19/2023 4:42 PM, Michael Niedermayer wrote: > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > > --- > > > > libavcodec/snowenc.c | 10 +++++++++- > > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c > > > > index 658684c575..5fb5906ed8 100644 > > > > --- a/libavcodec/snowenc.c > > > > +++ b/libavcodec/snowenc.c > > > > @@ -26,6 +26,7 @@ > > > > #include "avcodec.h" > > > > #include "codec_internal.h" > > > > #include "encode.h" > > > > +#include "internal.h" //For AVCodecInternal.recon_frame > > > > #include "me_cmp.h" > > > > #include "packet_internal.h" > > > > #include "snow_dwt.h" > > > > @@ -1576,6 +1577,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt, > > > > { > > > > SnowContext *s = avctx->priv_data; > > > > RangeCoder * const c= &s->c; > > > > + AVCodecInternal *avci = avctx->internal; > > > > AVFrame *pic; > > > > const int width= s->avctx->width; > > > > const int height= s->avctx->height; > > > > @@ -1877,6 +1879,10 @@ redo_frame: > > > > s->encoding_error, > > > > (s->avctx->flags&AV_CODEC_FLAG_PSNR) ? SNOW_MAX_PLANES : 0, > > > > s->current_picture->pict_type); > > > > + if (s->avctx->flags & AV_CODEC_FLAG_RECON_FRAME) { > > > > + av_frame_unref(avci->recon_frame); > > > > + av_frame_ref(avci->recon_frame, s->current_picture); > > > > + } > > > > pkt->size = ff_rac_terminate(c, 0); > > > > if (s->current_picture->key_frame) > > > > @@ -1934,7 +1940,9 @@ const FFCodec ff_snow_encoder = { > > > > CODEC_LONG_NAME("Snow"), > > > > .p.type = AVMEDIA_TYPE_VIDEO, > > > > .p.id = AV_CODEC_ID_SNOW, > > > > - .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE, > > > > + .p.capabilities = AV_CODEC_CAP_DR1 | > > > > + AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE | > > > > + AV_CODEC_CAP_ENCODER_RECON_FRAME, > > > > .priv_data_size = sizeof(SnowContext), > > > > .init = encode_init, > > > > FF_CODEC_ENCODE_CB(encode_frame), > > > > > > Testing with the tool from https://git.khirnov.net/libav.git/commit/?h=recon&id=d78e17840347abfe10b2bc7e789c60665de56e1f > > > (Which i hope will be upstreamed eventually) i get > > > > > > $ tools/enc_recon_frame_test input.y4m snow flags=+bitexact 10 > > > Checksum mismatch in frame ts=0, plane 0 > > > > I cannot replicate this, using matrixbench and above command line i get no > > mismatches, where can i find the input.y4m you used ? > > It was https://0x0.st/sfRh.y4m > > > > > also would be nice if this tool would write a image showing the difference > > > > thx > > > > > > > > I don't know if this really matters for snow (is it mean to be bitexact?), > > > but it seems the very first frame returned by the reconstructed frame API is > > > not the same as the one output by the snow decoder. The rest seem to match, > > > though. > > > > This sounds rather strange, the first frame would be used as reference for > > motion compensation so the subsequent frames matching after a mismatch is > > "surprising" > > I tried other supported encoders like libx264 and librav1e and none of them > report mismatches. bugs ill send a patchset thx [...]
On Fri, Mar 24, 2023 at 01:08:56AM +0100, Michael Niedermayer wrote: > On Thu, Mar 23, 2023 at 08:32:48AM -0300, James Almer wrote: > > On 3/23/2023 6:50 AM, Michael Niedermayer wrote: > > > On Mon, Mar 20, 2023 at 07:07:59PM -0300, James Almer wrote: > > > > On 3/19/2023 4:42 PM, Michael Niedermayer wrote: > > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> > > > > > --- > > > > > libavcodec/snowenc.c | 10 +++++++++- > > > > > 1 file changed, 9 insertions(+), 1 deletion(-) > > > > > > > > > > diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c > > > > > index 658684c575..5fb5906ed8 100644 > > > > > --- a/libavcodec/snowenc.c > > > > > +++ b/libavcodec/snowenc.c > > > > > @@ -26,6 +26,7 @@ > > > > > #include "avcodec.h" > > > > > #include "codec_internal.h" > > > > > #include "encode.h" > > > > > +#include "internal.h" //For AVCodecInternal.recon_frame > > > > > #include "me_cmp.h" > > > > > #include "packet_internal.h" > > > > > #include "snow_dwt.h" > > > > > @@ -1576,6 +1577,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt, > > > > > { > > > > > SnowContext *s = avctx->priv_data; > > > > > RangeCoder * const c= &s->c; > > > > > + AVCodecInternal *avci = avctx->internal; > > > > > AVFrame *pic; > > > > > const int width= s->avctx->width; > > > > > const int height= s->avctx->height; > > > > > @@ -1877,6 +1879,10 @@ redo_frame: > > > > > s->encoding_error, > > > > > (s->avctx->flags&AV_CODEC_FLAG_PSNR) ? SNOW_MAX_PLANES : 0, > > > > > s->current_picture->pict_type); > > > > > + if (s->avctx->flags & AV_CODEC_FLAG_RECON_FRAME) { > > > > > + av_frame_unref(avci->recon_frame); > > > > > + av_frame_ref(avci->recon_frame, s->current_picture); > > > > > + } > > > > > pkt->size = ff_rac_terminate(c, 0); > > > > > if (s->current_picture->key_frame) > > > > > @@ -1934,7 +1940,9 @@ const FFCodec ff_snow_encoder = { > > > > > CODEC_LONG_NAME("Snow"), > > > > > .p.type = AVMEDIA_TYPE_VIDEO, > > > > > .p.id = AV_CODEC_ID_SNOW, > > > > > - .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE, > > > > > + .p.capabilities = AV_CODEC_CAP_DR1 | > > > > > + AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE | > > > > > + AV_CODEC_CAP_ENCODER_RECON_FRAME, > > > > > .priv_data_size = sizeof(SnowContext), > > > > > .init = encode_init, > > > > > FF_CODEC_ENCODE_CB(encode_frame), > > > > > > > > Testing with the tool from https://git.khirnov.net/libav.git/commit/?h=recon&id=d78e17840347abfe10b2bc7e789c60665de56e1f > > > > (Which i hope will be upstreamed eventually) i get > > > > > > > > $ tools/enc_recon_frame_test input.y4m snow flags=+bitexact 10 > > > > Checksum mismatch in frame ts=0, plane 0 > > > > > > I cannot replicate this, using matrixbench and above command line i get no > > > mismatches, where can i find the input.y4m you used ? > > > > It was https://0x0.st/sfRh.y4m > > > > > > > > also would be nice if this tool would write a image showing the difference > > > > > > thx > > > > > > > > > > > I don't know if this really matters for snow (is it mean to be bitexact?), > > > > but it seems the very first frame returned by the reconstructed frame API is > > > > not the same as the one output by the snow decoder. The rest seem to match, > > > > though. > > > > > > This sounds rather strange, the first frame would be used as reference for > > > motion compensation so the subsequent frames matching after a mismatch is > > > "surprising" > > > > I tried other supported encoders like libx264 and librav1e and none of them > > report mismatches. > > bugs > ill send a patchset will apply the fix as well as these 3 patches here [...]
diff --git a/libavcodec/snowenc.c b/libavcodec/snowenc.c index 658684c575..5fb5906ed8 100644 --- a/libavcodec/snowenc.c +++ b/libavcodec/snowenc.c @@ -26,6 +26,7 @@ #include "avcodec.h" #include "codec_internal.h" #include "encode.h" +#include "internal.h" //For AVCodecInternal.recon_frame #include "me_cmp.h" #include "packet_internal.h" #include "snow_dwt.h" @@ -1576,6 +1577,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt, { SnowContext *s = avctx->priv_data; RangeCoder * const c= &s->c; + AVCodecInternal *avci = avctx->internal; AVFrame *pic; const int width= s->avctx->width; const int height= s->avctx->height; @@ -1877,6 +1879,10 @@ redo_frame: s->encoding_error, (s->avctx->flags&AV_CODEC_FLAG_PSNR) ? SNOW_MAX_PLANES : 0, s->current_picture->pict_type); + if (s->avctx->flags & AV_CODEC_FLAG_RECON_FRAME) { + av_frame_unref(avci->recon_frame); + av_frame_ref(avci->recon_frame, s->current_picture); + } pkt->size = ff_rac_terminate(c, 0); if (s->current_picture->key_frame) @@ -1934,7 +1940,9 @@ const FFCodec ff_snow_encoder = { CODEC_LONG_NAME("Snow"), .p.type = AVMEDIA_TYPE_VIDEO, .p.id = AV_CODEC_ID_SNOW, - .p.capabilities = AV_CODEC_CAP_DR1 | AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE, + .p.capabilities = AV_CODEC_CAP_DR1 | + AV_CODEC_CAP_ENCODER_REORDERED_OPAQUE | + AV_CODEC_CAP_ENCODER_RECON_FRAME, .priv_data_size = sizeof(SnowContext), .init = encode_init, FF_CODEC_ENCODE_CB(encode_frame),
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc> --- libavcodec/snowenc.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)