[FFmpeg-devel] [PATCH 1/3] avcodec/libwebpenc_animencoder: Don't return pkt without data/side-data

[James Almer]

On 4/10/2021 10:28 PM, Andreas Rheinhardt wrote:
> James Almer:
>> On 4/10/2021 9:39 PM, Andreas Rheinhardt wrote:
>>> They are essentially forbidden in our encoding API as they are
>>> considered empty. So just set the data, but leave the size at zero.
>>
>> This doesn't seem like a good solution. You're propagating dummy packets
>> when the encoder didn't produce any. It's an ugly hack to workaround a
>> misbehaving muxer.
>> As this is a AV_CODEC_CAP_DELAY encoder, you're supposed to set
>> got_packet to 0 in this scenario.
>>
>> The muxer should then not expect this encoder to return more than one
>> packet, which is all it ever truly produces, to assume it effectively
>> came from it.
>>
> 
> Then feel free to fix this yourself. All I know is that the muxer
> explicitly checks for the number of frames to be > 1. Not that I know
> why. (And I also presume the "Note: without the following, avio only
> writes 8 bytes to the file" to be outdated.)

I assume the muxer expected more than one packet because that's what the 
encoder produced, even if only the last one was legit. It nonetheless is 
a weird check, since for the purpose of writing the loop value it makes 
no difference whatsoever if the muxer got one or several packets, only 
that the is_animated_webp_packet() check succeeded, and that won't 
happen if a non animated webp image was fed.

> 
>>>
>>> (The old encoding API allowed such packets: It used buffer_pkt_valid
>>> to record whether the packet is empty or not. This has been changed
>>> in 827d6fe73d2f5472c1c2128eb14fab6a4db29032 which broke said encoder.
>>> Said regression has been reported in #9179 which this commit partially
>>> fixes.)
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at outlook.com>
>>> ---
>>>  Â  libavcodec/libwebpenc_animencoder.c | 3 +++
>>>  Â  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/libavcodec/libwebpenc_animencoder.c
>>> b/libavcodec/libwebpenc_animencoder.c
>>> index 7f35a0b939..d871e85d43 100644
>>> --- a/libavcodec/libwebpenc_animencoder.c
>>> +++ b/libavcodec/libwebpenc_animencoder.c
>>> @@ -34,6 +34,7 @@ typedef struct LibWebPAnimContext {
>>>  Â Â Â Â Â  WebPAnimEncoder *enc;Â Â Â Â  // the main AnimEncoder object
>>>  Â Â Â Â Â  int64_t prev_frame_pts;Â Â  // pts of the previously encoded frame.
>>>  Â Â Â Â Â  int done;Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â  // If true, we have assembled the
>>> bitstream already
>>> +Â Â Â  uint8_t padding_buf[AV_INPUT_BUFFER_PADDING_SIZE];
>>>  Â  } LibWebPAnimContext;
>>>  Â  Â  static av_cold int libwebp_anim_encode_init(AVCodecContext *avctx)
>>> @@ -103,6 +104,8 @@ static int
>>> libwebp_anim_encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>>>  Â Â Â Â Â Â Â Â Â  }
>>>  Â  Â Â Â Â Â Â Â Â Â  pkt->pts = pkt->dts = frame->pts;
>>> +Â Â Â Â Â Â Â  // Packets without data and side-data are not supported by
>>> the API
>>> +Â Â Â Â Â Â Â  pkt->data = s->padding_buf;
>>>  Â Â Â Â Â Â Â Â Â  s->prev_frame_pts = frame->pts;Â  // Save for next frame.
>>>  Â Â Â Â Â Â Â Â Â  ret = 0;
>>>  Â Â Â Â Â Â Â Â Â  *got_packet = 1;
>>>
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>