| Message ID | HE1PR0301MB2154E5A8AF65ECA5095F63028F719@HE1PR0301MB2154.eurprd03.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avcodec/libwebpenc_animencoder: Don't return pkt without data/side-data | expand |
| 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 |
On 4/10/2021 9:43 PM, Andreas Rheinhardt wrote: > When writing the trailer, the WebP muxer unconditionally seeks back > to the front to update some elements. Yet this doesn't work if > the seek failed, so check for this. > > (This has been mentioned in ticket #9179.) > > Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> > --- > libavformat/webpenc.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/libavformat/webpenc.c b/libavformat/webpenc.c > index 3962986c32..a24920d181 100644 > --- a/libavformat/webpenc.c > +++ b/libavformat/webpenc.c > @@ -174,8 +174,8 @@ static int webp_write_trailer(AVFormatContext *s) > > if (w->using_webp_anim_encoder) { > if ((w->frame_count > 1) && w->loop) { // Write loop count. > - avio_seek(s->pb, 42, SEEK_SET); > - avio_wl16(s->pb, w->loop); > + if (avio_seek(s->pb, 42, SEEK_SET) == 42) I think it's better if you also check for (s->pb->seekable & AVIO_SEEKABLE_NORMAL) before calling avio_seek() + avio_w* to begin with. > + avio_wl16(s->pb, w->loop); > } > } else { > int ret; > @@ -183,8 +183,8 @@ static int webp_write_trailer(AVFormatContext *s) > return ret; > > filesize = avio_tell(s->pb); > - avio_seek(s->pb, 4, SEEK_SET); > - avio_wl32(s->pb, filesize - 8); > + if (avio_seek(s->pb, 4, SEEK_SET) == 4) > + avio_wl32(s->pb, filesize - 8); > // Note: without the following, avio only writes 8 bytes to the file. > avio_seek(s->pb, filesize, SEEK_SET); > } >
James Almer: > On 4/10/2021 9:43 PM, Andreas Rheinhardt wrote: >> When writing the trailer, the WebP muxer unconditionally seeks back >> to the front to update some elements. Yet this doesn't work if >> the seek failed, so check for this. >> >> (This has been mentioned in ticket #9179.) >> >> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >> --- >> libavformat/webpenc.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/libavformat/webpenc.c b/libavformat/webpenc.c >> index 3962986c32..a24920d181 100644 >> --- a/libavformat/webpenc.c >> +++ b/libavformat/webpenc.c >> @@ -174,8 +174,8 @@ static int webp_write_trailer(AVFormatContext *s) >> if (w->using_webp_anim_encoder) { >> if ((w->frame_count > 1) && w->loop) { // Write loop count. >> - avio_seek(s->pb, 42, SEEK_SET); >> - avio_wl16(s->pb, w->loop); >> + if (avio_seek(s->pb, 42, SEEK_SET) == 42) > > I think it's better if you also check for (s->pb->seekable & > AVIO_SEEKABLE_NORMAL) before calling avio_seek() + avio_w* to begin with. > Actually I intentionally didn't do that because the seek might work even when said flag is not set (when the destination is still in the AVIOContext's buffer). >> + avio_wl16(s->pb, w->loop); >> } >> } else { >> int ret; >> @@ -183,8 +183,8 @@ static int webp_write_trailer(AVFormatContext *s) >> return ret; >> filesize = avio_tell(s->pb); >> - avio_seek(s->pb, 4, SEEK_SET); >> - avio_wl32(s->pb, filesize - 8); >> + if (avio_seek(s->pb, 4, SEEK_SET) == 4) >> + avio_wl32(s->pb, filesize - 8); >> // Note: without the following, avio only writes 8 bytes to >> the file. >> avio_seek(s->pb, filesize, SEEK_SET); >> } >> >
On 4/10/2021 10:21 PM, Andreas Rheinhardt wrote: > James Almer: >> On 4/10/2021 9:43 PM, Andreas Rheinhardt wrote: >>> When writing the trailer, the WebP muxer unconditionally seeks back >>> to the front to update some elements. Yet this doesn't work if >>> the seek failed, so check for this. >>> >>> (This has been mentioned in ticket #9179.) >>> >>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> >>> --- >>> libavformat/webpenc.c | 8 ++++---- >>> 1 file changed, 4 insertions(+), 4 deletions(-) >>> >>> diff --git a/libavformat/webpenc.c b/libavformat/webpenc.c >>> index 3962986c32..a24920d181 100644 >>> --- a/libavformat/webpenc.c >>> +++ b/libavformat/webpenc.c >>> @@ -174,8 +174,8 @@ static int webp_write_trailer(AVFormatContext *s) >>> if (w->using_webp_anim_encoder) { >>> if ((w->frame_count > 1) && w->loop) { // Write loop count. >>> - avio_seek(s->pb, 42, SEEK_SET); >>> - avio_wl16(s->pb, w->loop); >>> + if (avio_seek(s->pb, 42, SEEK_SET) == 42) >> >> I think it's better if you also check for (s->pb->seekable & >> AVIO_SEEKABLE_NORMAL) before calling avio_seek() + avio_w* to begin with. >> > > Actually I intentionally didn't do that because the seek might work even > when said flag is not set (when the destination is still in the > AVIOContext's buffer). Ok, patch LGTM then. Carl argued this value should not be set in webp_write_trailer() to begin with, but that's a separate change. > >>> + avio_wl16(s->pb, w->loop); >>> } >>> } else { >>> int ret; >>> @@ -183,8 +183,8 @@ static int webp_write_trailer(AVFormatContext *s) >>> return ret; >>> filesize = avio_tell(s->pb); >>> - avio_seek(s->pb, 4, SEEK_SET); >>> - avio_wl32(s->pb, filesize - 8); >>> + if (avio_seek(s->pb, 4, SEEK_SET) == 4) >>> + avio_wl32(s->pb, filesize - 8); >>> // Note: without the following, avio only writes 8 bytes to >>> the file. >>> avio_seek(s->pb, filesize, SEEK_SET); >>> } >>> >> > _______________________________________________ > 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 --git a/libavformat/webpenc.c b/libavformat/webpenc.c index 3962986c32..a24920d181 100644 --- a/libavformat/webpenc.c +++ b/libavformat/webpenc.c @@ -174,8 +174,8 @@ static int webp_write_trailer(AVFormatContext *s) if (w->using_webp_anim_encoder) { if ((w->frame_count > 1) && w->loop) { // Write loop count. - avio_seek(s->pb, 42, SEEK_SET); - avio_wl16(s->pb, w->loop); + if (avio_seek(s->pb, 42, SEEK_SET) == 42) + avio_wl16(s->pb, w->loop); } } else { int ret; @@ -183,8 +183,8 @@ static int webp_write_trailer(AVFormatContext *s) return ret; filesize = avio_tell(s->pb); - avio_seek(s->pb, 4, SEEK_SET); - avio_wl32(s->pb, filesize - 8); + if (avio_seek(s->pb, 4, SEEK_SET) == 4) + avio_wl32(s->pb, filesize - 8); // Note: without the following, avio only writes 8 bytes to the file. avio_seek(s->pb, filesize, SEEK_SET); }
When writing the trailer, the WebP muxer unconditionally seeks back to the front to update some elements. Yet this doesn't work if the seek failed, so check for this. (This has been mentioned in ticket #9179.) Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com> --- libavformat/webpenc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)