Re: [FFmpeg-devel] [PATCH]lavf/img2dec: Split img2 and img2pipe options
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 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
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 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
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