Re: [FFmpeg-devel] [PATCH]lavf/img2dec: Split img2 and img2pipe options

2019-02-10 Thread Gyan



On 10-02-2019 11:31 PM, Carl Eugen Hoyos wrote:

2019-02-10 18:44 GMT+01:00, Michael Niedermayer :

On Sat, Feb 09, 2019 at 01:58:06PM +0100, Carl Eugen Hoyos wrote:

Hi!

Currently if the "loop" option gets specified for -f image2pipe, there is
no indication for the user that the option will not work, patch attached.

Please comment, Carl Eugen
  img2dec.c |   33 -
  1 file changed, 20 insertions(+), 13 deletions(-)
eb41db12f922fc0780a808df6d8f832620ef9d65
0001-lavf-img2dec-Split-img2-and-img2pipe-options.patch
 From 21b11da9af5166e4bda7398a471a1e1a49352ac3 Mon Sep 17 00:00:00 2001
From: Carl Eugen Hoyos 
Date: Sat, 9 Feb 2019 13:49:51 +0100
Subject: [PATCH] lavf/img2dec: Split img2 and img2pipe options.

---
  libavformat/img2dec.c |   33 -
  1 file changed, 20 insertions(+), 13 deletions(-)

this breaks -loop

./ffmpeg -loop 1 -i ~/fatesamples/png1/lena-rgb24.png -t 1 test.avi

I moved the option that is obviously needed for the auto-detecting demuxers.

The original issue was that the image2pipe demuxer happily accepts
"loop" but ignores it - confusing users,
I suggest to keep ignoring it and print a warning for is_pipe && loop in 
read_header.


Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/img2dec: Split img2 and img2pipe options

2019-02-10 Thread Carl Eugen Hoyos
2019-02-10 18:44 GMT+01:00, Michael Niedermayer :
> On Sat, Feb 09, 2019 at 01:58:06PM +0100, Carl Eugen Hoyos wrote:
>> Hi!
>>
>> Currently if the "loop" option gets specified for -f image2pipe, there is
>> no indication for the user that the option will not work, patch attached.
>>
>> Please comment, Carl Eugen
>
>>  img2dec.c |   33 -
>>  1 file changed, 20 insertions(+), 13 deletions(-)
>> eb41db12f922fc0780a808df6d8f832620ef9d65
>> 0001-lavf-img2dec-Split-img2-and-img2pipe-options.patch
>> From 21b11da9af5166e4bda7398a471a1e1a49352ac3 Mon Sep 17 00:00:00 2001
>> From: Carl Eugen Hoyos 
>> Date: Sat, 9 Feb 2019 13:49:51 +0100
>> Subject: [PATCH] lavf/img2dec: Split img2 and img2pipe options.
>>
>> ---
>>  libavformat/img2dec.c |   33 -
>>  1 file changed, 20 insertions(+), 13 deletions(-)
>
> this breaks -loop
>
> ./ffmpeg -loop 1 -i ~/fatesamples/png1/lena-rgb24.png -t 1 test.avi

I moved the option that is obviously needed for the auto-detecting demuxers.

The original issue was that the image2pipe demuxer happily accepts
"loop" but ignores it - confusing users, I wonder if the only solution is
to split the options between image2pipe and the auto-detecting
demuxers or if there is another solution (or if both patches should
be reverted).

Sorry, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/img2dec: Split img2 and img2pipe options

2019-02-10 Thread Michael Niedermayer
On Sat, Feb 09, 2019 at 01:58:06PM +0100, Carl Eugen Hoyos wrote:
> Hi!
> 
> Currently if the "loop" option gets specified for -f image2pipe, there is
> no indication for the user that the option will not work, patch attached.
> 
> Please comment, Carl Eugen

>  img2dec.c |   33 -
>  1 file changed, 20 insertions(+), 13 deletions(-)
> eb41db12f922fc0780a808df6d8f832620ef9d65  
> 0001-lavf-img2dec-Split-img2-and-img2pipe-options.patch
> From 21b11da9af5166e4bda7398a471a1e1a49352ac3 Mon Sep 17 00:00:00 2001
> From: Carl Eugen Hoyos 
> Date: Sat, 9 Feb 2019 13:49:51 +0100
> Subject: [PATCH] lavf/img2dec: Split img2 and img2pipe options.
> 
> ---
>  libavformat/img2dec.c |   33 -
>  1 file changed, 20 insertions(+), 13 deletions(-)

this breaks -loop

./ffmpeg -loop 1 -i ~/fatesamples/png1/lena-rgb24.png -t 1 test.avi

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/img2dec: Split img2 and img2pipe options

2019-02-10 Thread Carl Eugen Hoyos
2019-02-10 1:55 GMT+01:00, Jan Ekström :
> On Sat, Feb 9, 2019 at 2:58 PM Carl Eugen Hoyos  wrote:
>>
>> Hi!
>>
>> Currently if the "loop" option gets specified for -f image2pipe, there is
>> no indication for the user that the option will not work, patch attached.
>>
>> Please comment, Carl Eugen
>
> Seems like a good idea.
>
> The reason for the pipe version's AVOption structure to be outside of
> the if is the IMAGEAUTO_DEMUXER macro, so I guess unless we like
> really long #ifs then this is as good as it gets.

An alternative is to move the options into the IMAGEAUTO_DEMUXER
macro but I did not see an advantage.

> Quickly checked and it seems like generally the separation of options
> seems correct... but I'm not sure about "ts_from_file" . The filename
> variable will probably be full of \0s at the point where stat() is
> called, as it's only set to something else under "!s->is_pipe" branch
> in the beginning of the function if I'm seeing correctly - thus making
> this option effectively unusable under the _pipe demuxers?

Definitely, I left ts_from_file where it was and pushed.

Thank you, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel


Re: [FFmpeg-devel] [PATCH]lavf/img2dec: Split img2 and img2pipe options

2019-02-09 Thread Jan Ekström
On Sat, Feb 9, 2019 at 2:58 PM Carl Eugen Hoyos  wrote:
>
> Hi!
>
> Currently if the "loop" option gets specified for -f image2pipe, there is
> no indication for the user that the option will not work, patch attached.
>
> Please comment, Carl Eugen

Seems like a good idea.

The reason for the pipe version's AVOption structure to be outside of
the if is the IMAGEAUTO_DEMUXER macro, so I guess unless we like
really long #ifs then this is as good as it gets.

Quickly checked and it seems like generally the separation of options
seems correct... but I'm not sure about "ts_from_file" . The filename
variable will probably be full of \0s at the point where stat() is
called, as it's only set to something else under "!s->is_pipe" branch
in the beginning of the function if I'm seeing correctly - thus making
this option effectively unusable under the _pipe demuxers?

Jan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel