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

[Alexander Strasser]

On 2021-01-09 16:27 +0100, Reimar Döffinger wrote:
> 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.

Seems it didn't (and probably still doesn't) work like we expected.

I'm fine with your patch as it was tested that way.


  Alexander