diff mbox series

[FFmpeg-devel,3/3] avformat/webpenc: don't assume animated webp streams will have more than one packet

Message ID 20210411021238.29014-3-jamrial@gmail.com
State Accepted
Commit 55d667d86af7d13fc5aa2b4071a5b97eb10e2da6
Headers show
Series [FFmpeg-devel,1/3] avcodec/libwebpenc_animencoder: stop propagating bogus empty packets
Related show

Checks

Context Check Description
andriy/x86_make success Make finished
andriy/x86_make_fate success Make fate finished
andriy/PPC64_make success Make finished
andriy/PPC64_make_fate success Make fate finished

Commit Message

James Almer April 11, 2021, 2:12 a.m. UTC
The libwebp_animencoder returns a single packet with the entire animated
stream, as that's what the external library produces. As such, only ensure the
stream was produced by said encoder (or propagated by a demuxer, once support
is added) when attempting to write the requested loop value.

Fixes ticket #9179.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavformat/webpenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Carl Eugen Hoyos April 11, 2021, 4:30 p.m. UTC | #1
Am So., 11. Apr. 2021 um 04:13 Uhr schrieb James Almer <jamrial@gmail.com>:
>
> The libwebp_animencoder returns a single packet with the entire animated
> stream, as that's what the external library produces. As such, only ensure the
> stream was produced by said encoder (or propagated by a demuxer, once support
> is added) when attempting to write the requested loop value.
>
> Fixes ticket #9179.
>
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavformat/webpenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/webpenc.c b/libavformat/webpenc.c
> index ed8325c02d..ca4ffc4e2d 100644
> --- a/libavformat/webpenc.c
> +++ b/libavformat/webpenc.c
> @@ -172,7 +172,7 @@ static int webp_write_trailer(AVFormatContext *s)
>      WebpContext *w = s->priv_data;
>
>      if (w->using_webp_anim_encoder) {
> -        if ((w->frame_count > 1) && w->loop) {  // Write loop count.
> +        if (w->loop) {  // Write loop count.
>              avio_seek(s->pb, 42, SEEK_SET);
>              avio_wl16(s->pb, w->loop);

Sorry for not understanding this:
Why is the seek necessary at all?

Carl Eugen
James Almer April 11, 2021, 4:50 p.m. UTC | #2
On 4/11/2021 1:30 PM, Carl Eugen Hoyos wrote:
> Am So., 11. Apr. 2021 um 04:13 Uhr schrieb James Almer <jamrial@gmail.com>:
>>
>> The libwebp_animencoder returns a single packet with the entire animated
>> stream, as that's what the external library produces. As such, only ensure the
>> stream was produced by said encoder (or propagated by a demuxer, once support
>> is added) when attempting to write the requested loop value.
>>
>> Fixes ticket #9179.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavformat/webpenc.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavformat/webpenc.c b/libavformat/webpenc.c
>> index ed8325c02d..ca4ffc4e2d 100644
>> --- a/libavformat/webpenc.c
>> +++ b/libavformat/webpenc.c
>> @@ -172,7 +172,7 @@ static int webp_write_trailer(AVFormatContext *s)
>>       WebpContext *w = s->priv_data;
>>
>>       if (w->using_webp_anim_encoder) {
>> -        if ((w->frame_count > 1) && w->loop) {  // Write loop count.
>> +        if (w->loop) {  // Write loop count.
>>               avio_seek(s->pb, 42, SEEK_SET);
>>               avio_wl16(s->pb, w->loop);
> 
> Sorry for not understanding this:
> Why is the seek necessary at all?

If we write the loop value in webp_write_trailer() then there's no other 
way. But if we instead attempt to write it in webp_write_packet() then a 
seek wouldn't be needed, no.

> 
> Carl Eugen
> _______________________________________________
> 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".
>
Carl Eugen Hoyos April 11, 2021, 5:15 p.m. UTC | #3
Am So., 11. Apr. 2021 um 18:50 Uhr schrieb James Almer <jamrial@gmail.com>:
>
> On 4/11/2021 1:30 PM, Carl Eugen Hoyos wrote:
> > Am So., 11. Apr. 2021 um 04:13 Uhr schrieb James Almer <jamrial@gmail.com>:
> >>
> >> The libwebp_animencoder returns a single packet with the entire animated
> >> stream, as that's what the external library produces. As such, only ensure the
> >> stream was produced by said encoder (or propagated by a demuxer, once support
> >> is added) when attempting to write the requested loop value.
> >>
> >> Fixes ticket #9179.
> >>
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >>   libavformat/webpenc.c | 2 +-
> >>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavformat/webpenc.c b/libavformat/webpenc.c
> >> index ed8325c02d..ca4ffc4e2d 100644
> >> --- a/libavformat/webpenc.c
> >> +++ b/libavformat/webpenc.c
> >> @@ -172,7 +172,7 @@ static int webp_write_trailer(AVFormatContext *s)
> >>       WebpContext *w = s->priv_data;
> >>
> >>       if (w->using_webp_anim_encoder) {
> >> -        if ((w->frame_count > 1) && w->loop) {  // Write loop count.
> >> +        if (w->loop) {  // Write loop count.
> >>               avio_seek(s->pb, 42, SEEK_SET);
> >>               avio_wl16(s->pb, w->loop);
> >
> > Sorry for not understanding this:
> > Why is the seek necessary at all?
>
> If we write the loop value in webp_write_trailer() then there's no other
> way. But if we instead attempt to write it in webp_write_packet() then a
> seek wouldn't be needed, no.

If I don't miss anything, the value should not be written in
webp_write_trailer() then.

Carl Eugen
James Almer April 11, 2021, 5:19 p.m. UTC | #4
On 4/11/2021 2:15 PM, Carl Eugen Hoyos wrote:
> Am So., 11. Apr. 2021 um 18:50 Uhr schrieb James Almer <jamrial@gmail.com>:
>>
>> On 4/11/2021 1:30 PM, Carl Eugen Hoyos wrote:
>>> Am So., 11. Apr. 2021 um 04:13 Uhr schrieb James Almer <jamrial@gmail.com>:
>>>>
>>>> The libwebp_animencoder returns a single packet with the entire animated
>>>> stream, as that's what the external library produces. As such, only ensure the
>>>> stream was produced by said encoder (or propagated by a demuxer, once support
>>>> is added) when attempting to write the requested loop value.
>>>>
>>>> Fixes ticket #9179.
>>>>
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---
>>>>    libavformat/webpenc.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/libavformat/webpenc.c b/libavformat/webpenc.c
>>>> index ed8325c02d..ca4ffc4e2d 100644
>>>> --- a/libavformat/webpenc.c
>>>> +++ b/libavformat/webpenc.c
>>>> @@ -172,7 +172,7 @@ static int webp_write_trailer(AVFormatContext *s)
>>>>        WebpContext *w = s->priv_data;
>>>>
>>>>        if (w->using_webp_anim_encoder) {
>>>> -        if ((w->frame_count > 1) && w->loop) {  // Write loop count.
>>>> +        if (w->loop) {  // Write loop count.
>>>>                avio_seek(s->pb, 42, SEEK_SET);
>>>>                avio_wl16(s->pb, w->loop);
>>>
>>> Sorry for not understanding this:
>>> Why is the seek necessary at all?
>>
>> If we write the loop value in webp_write_trailer() then there's no other
>> way. But if we instead attempt to write it in webp_write_packet() then a
>> seek wouldn't be needed, no.
> 
> If I don't miss anything, the value should not be written in
> webp_write_trailer() then.

Patch welcome, of course.

> 
> Carl Eugen
> _______________________________________________
> 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".
>
diff mbox series

Patch

diff --git a/libavformat/webpenc.c b/libavformat/webpenc.c
index ed8325c02d..ca4ffc4e2d 100644
--- a/libavformat/webpenc.c
+++ b/libavformat/webpenc.c
@@ -172,7 +172,7 @@  static int webp_write_trailer(AVFormatContext *s)
     WebpContext *w = s->priv_data;
 
     if (w->using_webp_anim_encoder) {
-        if ((w->frame_count > 1) && w->loop) {  // Write loop count.
+        if (w->loop) {  // Write loop count.
             avio_seek(s->pb, 42, SEEK_SET);
             avio_wl16(s->pb, w->loop);
         }