[MPlayer-dev-eng] [PATCH 1/6] vo_png: switch to new FFmpeg API.

[Reimar Döffinger]

On Sat, Jan 09, 2021 at 01:00:21AM +0100, Alexander Strasser wrote:
> On 2020-12-20 18:50 +0100, Reimar Döffinger wrote:
> > ---
> >  libvo/vo_png.c | 27 ++++++++++++---------------
> >  1 file changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/libvo/vo_png.c b/libvo/vo_png.c
> > index 5696a1f3e..7523dca74 100644
> > --- a/libvo/vo_png.c
> > +++ b/libvo/vo_png.c
> > @@ -150,8 +150,7 @@ config(uint32_t width, uint32_t height, uint32_t d_width, uint32_t d_height, uin
> >
> >  static uint32_t draw_image(mp_image_t* mpi){
> >      AVFrame *pic;
> > -    int buffersize;
> > -    int res, got_pkt;
> > +    int res;
> >      char buf[100];
> >      FILE *outfile;
> >      AVPacket pkt;
> > @@ -174,26 +173,25 @@ static uint32_t draw_image(mp_image_t* mpi){
> >      pic->format = imgfmt2pixfmt(png_format);
> >      pic->data[0] = mpi->planes[0];
> >      pic->linesize[0] = mpi->stride[0];
> > -    buffersize = mpi->w * mpi->h * 8;
> > -    if (outbuffer_size < buffersize) {
> > -        av_freep(&outbuffer);
> > -        outbuffer = av_malloc(buffersize);
> > -        outbuffer_size = buffersize;
> > -    }
> >      av_init_packet(&pkt);
> > -    pkt.data = outbuffer;
> > -    pkt.size = outbuffer_size;
>
> > -    res = avcodec_encode_video2(avctx, &pkt, pic, &got_pkt);
> > +    res = avcodec_send_frame(avctx, pic);
> > +    if (res >= 0) {
> > +        res = avcodec_receive_packet(avctx, &pkt);
> > +        if (res == AVERROR(EAGAIN)) {
> > +            avcodec_send_frame(avctx, NULL);
> > +            res = avcodec_receive_packet(avctx, &pkt);
> > +        }
> > +    }
>
> I don't like how this gets harder to read after the patch.
>
> Did you try it like this?
>
> 1, avcodec_send_frame(avctx, pic)
> 2. avcodec_send_frame(avctx, NULL)
> 3. avcodec_receive_packet(avctx, &pkt)
>

This is related to the decoder side.
I have not re-tested since to make sure, and it
certainly would be nice it worked that way, but what
I have seen for PNG codec is:
- once you do send_frame(NULL) the receive_packet will always return an
  error.
- the codec is then also stuck in this error state, so you cannot
  re-use the encoder even if you send in more real frames.
  There are some reset functions that I thought were for this
  use-case but I just got a warning that the encoder does not
  support it.
- for this use-case, that means you would never get any frame out.
- however, API-wise you are supposed to do the send_frame(NULL) per
  my understanding, so I added it in case FFmpeg changes/fixes it.

Now I could try to investigate and fix it in FFmpeg, but that comes
with the dual issue of me not having time, plus usually "make the
API less painful to use" type of patches usually encounter resistance
of the "you just shouldn't use it that way" type.