[FFmpeg-devel] [PATCH 1/3] avformat/file: check for lseek() failure

[FFmpeg-devel] [PATCH 1/3] avformat/file: check for lseek() failure

[Michael Niedermayer]

Fixes: CID1559855 Unchecked return value from library

Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/file.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/libavformat/file.c b/libavformat/file.c
index 0ed4cff266c..e2ef85aa705 100644
--- a/libavformat/file.c
+++ b/libavformat/file.c
@@ -222,8 +222,10 @@ static int file_close(URLContext *h)
     FileContext *c = h->priv_data;
     int ret;
 
-    if (c->initial_pos >= 0 && !h->is_streamed)
-        lseek(c->fd, c->initial_pos, SEEK_SET);
+    if (c->initial_pos >= 0 && !h->is_streamed) {
+        if (lseek(c->fd, c->initial_pos, SEEK_SET) < 0)
+            av_log(h, AV_LOG_DEBUG, "Failed to restore position (%s)\n", av_err2str(AVERROR(errno)));
+    }
 
     ret = close(c->fd);
     return (ret == -1) ? AVERROR(errno) : 0;
-- 
2.45.1

_______________________________________________
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".

[FFmpeg-devel] [PATCH 2/3] avformat/fwse: Remove always false expression

[Michael Niedermayer]

Fixes: CID1460758 Operands don't affect result

Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/fwse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/fwse.c b/libavformat/fwse.c
index 6c1103da146..dc4750d946a 100644
--- a/libavformat/fwse.c
+++ b/libavformat/fwse.c
@@ -67,7 +67,7 @@ static int fwse_read_header(AVFormatContext *s)
     av_channel_layout_default(&par->ch_layout, channels);
     st->duration = avio_rl32(pb);
     par->sample_rate = avio_rl32(pb);
-    if (par->sample_rate <= 0 || par->sample_rate > INT_MAX)
+    if (par->sample_rate <= 0)
         return AVERROR_INVALIDDATA;
 
     par->block_align = 1;
-- 
2.45.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/fwse: Remove always false expression

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 535 bytes --]

On Sat, May 25, 2024 at 01:18:13PM +0200, Michael Niedermayer wrote:
> Fixes: CID1460758 Operands don't affect result
> 
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/fwse.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

will apply

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

[FFmpeg-devel] [PATCH 3/3] avformat/hlsenc: Remove dead ret stores

[Michael Niedermayer]

Fixes: CID1529222 Unused value

Sponsored-by: Sovereign Tech Fund
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
---
 libavformat/hlsenc.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
index 0e2843c6bc5..8a43ef6232a 100644
--- a/libavformat/hlsenc.c
+++ b/libavformat/hlsenc.c
@@ -1582,8 +1582,6 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
     ret = hlsenc_io_open(s, byterange_mode ? &hls->m3u8_out : &vs->out, temp_filename, &options);
     av_dict_free(&options);
     if (ret < 0) {
-        if (hls->ignore_io_errors)
-            ret = 0;
         goto fail;
     }
 
@@ -1641,8 +1639,6 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
         ret = hlsenc_io_open(s, &hls->sub_m3u8_out, temp_vtt_filename, &options);
         av_dict_free(&options);
         if (ret < 0) {
-            if (hls->ignore_io_errors)
-                ret = 0;
             goto fail;
         }
         ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, hls->allowcache,
-- 
2.45.1

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 3/3] avformat/hlsenc: Remove dead ret stores

[Steven Liu]

Michael Niedermayer <michael@niedermayer.cc> 于2024年5月25日周六 19:18写道：

>
> Fixes: CID1529222 Unused value
>
> Sponsored-by: Sovereign Tech Fund
> Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> ---
>  libavformat/hlsenc.c | 4 ----
>  1 file changed, 4 deletions(-)
>
> diff --git a/libavformat/hlsenc.c b/libavformat/hlsenc.c
> index 0e2843c6bc5..8a43ef6232a 100644
> --- a/libavformat/hlsenc.c
> +++ b/libavformat/hlsenc.c
> @@ -1582,8 +1582,6 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>      ret = hlsenc_io_open(s, byterange_mode ? &hls->m3u8_out : &vs->out, temp_filename, &options);
>      av_dict_free(&options);
>      if (ret < 0) {
> -        if (hls->ignore_io_errors)
> -            ret = 0;
>          goto fail;
>      }
>
> @@ -1641,8 +1639,6 @@ static int hls_window(AVFormatContext *s, int last, VariantStream *vs)
>          ret = hlsenc_io_open(s, &hls->sub_m3u8_out, temp_vtt_filename, &options);
>          av_dict_free(&options);
>          if (ret < 0) {
> -            if (hls->ignore_io_errors)
> -                ret = 0;
>              goto fail;
>          }
>          ff_hls_write_playlist_header(hls->sub_m3u8_out, hls->version, hls->allowcache,
> --
> 2.45.1
>
> _______________________________________________
> 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".

LGTM

Thanks
Steven
_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/file: check for lseek() failure

[Rémi Denis-Courmont]

Le lauantaina 25. toukokuuta 2024, 14.18.12 EEST Michael Niedermayer a écrit :
> Fixes: CID1559855 Unchecked return value from library

This is again very misleading. Logging an error (that most people won't even 
see) is not fixing a bug. Either this is not a bug, and this patch can't be 
fixing a non-bug, or this is a bug, and this patch is hiding it behind a fake 
fix.

In this case, the code looks completely bogus. Restoring the file pointer 
before closing the descriptor makes no sense. In the first place, it can only 
have an observable effect if the file descriptor is borrowed (i.e. "fd" 
protocol). And even then, whatever external code relies on this peculiar 
behaviour is irremediably broken, as Coverity highlights. If this is really 
needed, the only way that this can work properly is for that external code to 
restore the file offset after libavformat is done, and check the error *there*.

Also, AFAIK, by the principle of least surprise, and following the behaviour 
of most other programs in similar circumstances, the expected behaviour is 
*not* to restore the file pointer.

So IMO, 5c3d2177e7be7dde69a83b1b394f57976c164c84 should be reverted instead.

-- 
レミ・デニ-クールモン
http://www.remlab.net/



_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/file: check for lseek() failure

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 899 bytes --]

On Sat, May 25, 2024 at 02:38:07PM +0300, Rémi Denis-Courmont wrote:
> Le lauantaina 25. toukokuuta 2024, 14.18.12 EEST Michael Niedermayer a écrit :
> > Fixes: CID1559855 Unchecked return value from library
> 
> This is again very misleading. Logging an error (that most people won't even 
> see) is not fixing a bug. Either this is not a bug, and this patch can't be 
> fixing a non-bug, or this is a bug, and this patch is hiding it behind a fake 
> fix.

As written in the commit message, the patch would fix CID1559855.
It doesnt claim anything else

[...]


> 
> So IMO, 5c3d2177e7be7dde69a83b1b394f57976c164c84 should be reverted instead.

if thats preferred, i have nothing against doing that instead

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Never trust a computer, one day, it may think you are the virus. -- Compn

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/file: check for lseek() failure

[Rémi Denis-Courmont]

Le lauantaina 25. toukokuuta 2024, 15.19.51 EEST Michael Niedermayer a écrit :
> On Sat, May 25, 2024 at 02:38:07PM +0300, Rémi Denis-Courmont wrote:
> > Le lauantaina 25. toukokuuta 2024, 14.18.12 EEST Michael Niedermayer a 
écrit :
> > > Fixes: CID1559855 Unchecked return value from library
> > 
> > This is again very misleading. Logging an error (that most people won't
> > even see) is not fixing a bug. Either this is not a bug, and this patch
> > can't be fixing a non-bug, or this is a bug, and this patch is hiding it
> > behind a fake fix.
> 
> As written in the commit message, the patch would fix CID1559855.
> It doesnt claim anything else

That does not match my understanding of the verb "to fix". And this is not just 
a rant about semantics. People may well be mislead and assume that any commit 
that "fixes" something fixes an FFmpeg bug.

> > So IMO, 5c3d2177e7be7dde69a83b1b394f57976c164c84 should be reverted
> > instead.
> if thats preferred, i have nothing against doing that instead
> 
> thx
> 
> [...]


-- 
レミ・デニ-クールモン
http://www.remlab.net/



_______________________________________________
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".