Re: [FFmpeg-devel] [PATCH]lavc/x264: Improve level setting
On Wed, Jan 27, 2016 at 09:35:42AM +, Carl Eugen Hoyos wrote:
> Carl Eugen Hoyos ag.or.at> writes:
>
> > if (!strcmp(x4->level, "1b")) {
> > level_id = 9;
> > +} else if (atoi(x4->level) > 9) {
> > +level_id = atoi(x4->level);
>
> Ping.
>
> If this patch is not ok, please explain for
> what input it behaves incorrectly.
it is "ok" but not robust
IIRC it depends on undocumented behavior of an external lib
that is libx264 checking the string ...
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Democracy is the form of government in which you can choose your dictator
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/x264: Improve level setting
Carl Eugen Hoyos ag.or.at> writes:
> if (!strcmp(x4->level, "1b")) {
> level_id = 9;
> +} else if (atoi(x4->level) > 9) {
> +level_id = atoi(x4->level);
Ping.
If this patch is not ok, please explain for
what input it behaves incorrectly.
Thank you, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/x264: Improve level setting
Carl Eugen Hoyos ag.or.at> writes: > Nicolas George nsup.org> writes: > > > Le nonidi 29 nivôse, an CCXXIV, Carl Eugen Hoyos a écrit : > > > The original patch (that does not care about tail > > > since it can't be reached anyway) uses atoi(). > > > Is that not ok? > > > > atoi() has undefined behaviours in case of error > > How can I produce an error here? Ping, this code can only be reached with small numbers, so please explain what input would be an issue. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/x264: Improve level setting
Nicolas George nsup.org> writes: > Le nonidi 29 nivôse, an CCXXIV, Carl Eugen Hoyos a écrit : > > The original patch (that does not care about tail > > since it can't be reached anyway) uses atoi(). > > Is that not ok? > > atoi() has undefined behaviours in case of error How can I produce an error here? > and has a trivial replacement that works best Please elaborate. Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/x264: Improve level setting
Le nonidi 29 nivôse, an CCXXIV, Carl Eugen Hoyos a écrit : > The original patch (that does not care about tail > since it can't be reached anyway) uses atoi(). > Is that not ok? atoi() has undefined behaviours in case of error, and has a trivial replacement that works best: it should not be used at all IMHO. Regards, -- Nicolas George signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/x264: Improve level setting
Michael Niedermayer niedermayer.cc> writes: > > > > > this should check that teres no "tail" after the integer > > > > > > > > For which command line will this make a difference? > > > > I didn't find one... > indeed, i guess x264 checks this internally before > our code gets a chance to fail So is the patch ok? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/x264: Improve level setting
On Sun, Jan 17, 2016 at 11:17:36PM +, Carl Eugen Hoyos wrote: > Michael Niedermayer niedermayer.cc> writes: > > > > > > > this should check that teres no "tail" after the integer > > > > > > > > > > For which command line will this make a difference? > > > > > I didn't find one... > > > indeed, i guess x264 checks this internally before > > our code gets a chance to fail > > So is the patch ok? id use strtol instead of strtod to parse a value that is expeted to be in integer, or is it not expected to be an integer ? either way, it should be ok -- 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 signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/x264: Improve level setting
Michael Niedermayer niedermayer.cc> writes: > > So is the patch ok? > > id use strtol instead of strtod to parse a value that > is expeted to be in integer, or is it not expected to > be an integer ? The original patch (that does not care about tail since it can't be reached anyway) uses atoi(). Is that not ok? Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/x264: Improve level setting
On Tue, Jan 12, 2016 at 02:39:35PM +0100, Carl Eugen Hoyos wrote: > On Tuesday 12 January 2016 02:31:48 pm Michael Niedermayer wrote: > > > > > +level_id = atoi(x4->level); > > > > > > > > this should check that teres no "tail" after the integer > > > > > > For which command line will this make a difference? > > > I didn't find one... > > > > didnt try but i was thinking of something like 11.8 > > ffmpeg -f lavfi -i testsrc=s=hd1080 -pix_fmt yuv420p -t 1 > -level 11.8 -preset veryslow out.mov > ffmpeg version N-77804-gd64d6ed Copyright (c) 2000-2016 the FFmpeg developers > built with gcc 4.7 (SUSE Linux) > configuration: --enable-gpl --enable-libx264 > libavutil 55. 13.100 / 55. 13.100 > libavcodec 57. 22.100 / 57. 22.100 > libavformat57. 21.101 / 57. 21.101 > libavdevice57. 0.100 / 57. 0.100 > libavfilter 6. 23.100 / 6. 23.100 > libswscale 4. 0.100 / 4. 0.100 > libswresample 2. 0.101 / 2. 0.101 > libpostproc54. 0.100 / 54. 0.100 > Input #0, lavfi, from 'testsrc=s=hd1080': > Duration: N/A, start: 0.00, bitrate: N/A > Stream #0:0: Video: rawvideo (RGB[24] / 0x18424752), rgb24, 1920x1080 > [SAR > 1:1 DAR 16:9], 25 tbr, 25 tbn, 25 tbc > [libx264 @ 0x29335c0] Error parsing option 'level' with value '11.8'. > Output #0, mov, to 'out.mov': > Stream #0:0: Video: h264, none, q=2-31, 128 kb/s, SAR 1:1 DAR 0:0, 25 fps > Metadata: > encoder : Lavc57.22.100 libx264 > Stream mapping: > Stream #0:0 -> #0:0 (rawvideo (native) -> h264 (libx264)) > Error while opening encoder for output stream #0:0 - maybe incorrect > parameters such as bit_rate, rate, width or height indeed, i guess x264 checks this internally before our code gets a chance to fail [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Awnsering whenever a program halts or runs forever is On a turing machine, in general impossible (turings halting problem). On any real computer, always possible as a real computer has a finite number of states N, and will either halt in less than N cycles or never halt. signature.asc Description: Digital signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/x264: Improve level setting
On Tuesday 12 January 2016 02:31:48 pm Michael Niedermayer wrote: > > > > +level_id = atoi(x4->level); > > > > > > this should check that teres no "tail" after the integer > > > > For which command line will this make a difference? > > I didn't find one... > > didnt try but i was thinking of something like 11.8 ffmpeg -f lavfi -i testsrc=s=hd1080 -pix_fmt yuv420p -t 1 -level 11.8 -preset veryslow out.mov ffmpeg version N-77804-gd64d6ed Copyright (c) 2000-2016 the FFmpeg developers built with gcc 4.7 (SUSE Linux) configuration: --enable-gpl --enable-libx264 libavutil 55. 13.100 / 55. 13.100 libavcodec 57. 22.100 / 57. 22.100 libavformat57. 21.101 / 57. 21.101 libavdevice57. 0.100 / 57. 0.100 libavfilter 6. 23.100 / 6. 23.100 libswscale 4. 0.100 / 4. 0.100 libswresample 2. 0.101 / 2. 0.101 libpostproc54. 0.100 / 54. 0.100 Input #0, lavfi, from 'testsrc=s=hd1080': Duration: N/A, start: 0.00, bitrate: N/A Stream #0:0: Video: rawvideo (RGB[24] / 0x18424752), rgb24, 1920x1080 [SAR 1:1 DAR 16:9], 25 tbr, 25 tbn, 25 tbc [libx264 @ 0x29335c0] Error parsing option 'level' with value '11.8'. Output #0, mov, to 'out.mov': Stream #0:0: Video: h264, none, q=2-31, 128 kb/s, SAR 1:1 DAR 0:0, 25 fps Metadata: encoder : Lavc57.22.100 libx264 Stream mapping: Stream #0:0 -> #0:0 (rawvideo (native) -> h264 (libx264)) Error while opening encoder for output stream #0:0 - maybe incorrect parameters such as bit_rate, rate, width or height Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/x264: Improve level setting
On Tue, Jan 12, 2016 at 02:10:08PM +0100, Carl Eugen Hoyos wrote:
> On Tuesday 12 January 2016 01:55:30 pm Michael Niedermayer wrote:
> > On Mon, Jan 11, 2016 at 10:58:55AM +0100, Carl Eugen Hoyos wrote:
> > > Hi!
> > >
> > > I guess that attached patch fixes the additional issue in ticket #3307.
> > >
> > > Please comment, Carl Eugen
> > >
> > > libx264.c |2 ++
> > > 1 file changed, 2 insertions(+)
> > > cea8163693a2a78b9dc2d1929082e7c76ac542ac patchx264level.diff
> > > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> > > index 88406a3..c1e52a1 100644
> > > --- a/libavcodec/libx264.c
> > > +++ b/libavcodec/libx264.c
> > > @@ -559,6 +559,8 @@ static av_cold int X264_init(AVCodecContext *avctx)
> > >
> > > if (!strcmp(x4->level, "1b")) {
> > > level_id = 9;
>
> > > +} else if (atoi(x4->level) > 9) {
>
> Should be "> 8".
>
> > > +level_id = atoi(x4->level);
> >
> > this should check that teres no "tail" after the integer
>
> For which command line will this make a difference?
> I didn't find one...
didnt try but i was thinking of something like 11.8
whatever the user meant by that it likely shouldnt silently be
interpreted as 1.1, that could be rather confusing
>
> New patch attached, Carl Eugen
> libx264.c |2 ++
> 1 file changed, 2 insertions(+)
> 00dbf2a3deee89c56aaa0743c3f5cb90f2e4b5e0 patchx264level.diff
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index 88406a3..abb6985 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -559,6 +559,8 @@ static av_cold int X264_init(AVCodecContext *avctx)
>
> if (!strcmp(x4->level, "1b")) {
> level_id = 9;
> +} else if (av_strtod(x4->level, ) > 8.0 && !*tail) {
strtol or something
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The bravest are surely those who have the clearest vision
of what is before them, glory and danger alike, and yet
notwithstanding go out to meet it. -- Thucydides
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/x264: Improve level setting
On Mon, Jan 11, 2016 at 10:58:55AM +0100, Carl Eugen Hoyos wrote:
> Hi!
>
> I guess that attached patch fixes the additional issue in ticket #3307.
>
> Please comment, Carl Eugen
> libx264.c |2 ++
> 1 file changed, 2 insertions(+)
> cea8163693a2a78b9dc2d1929082e7c76ac542ac patchx264level.diff
> diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> index 88406a3..c1e52a1 100644
> --- a/libavcodec/libx264.c
> +++ b/libavcodec/libx264.c
> @@ -559,6 +559,8 @@ static av_cold int X264_init(AVCodecContext *avctx)
>
> if (!strcmp(x4->level, "1b")) {
> level_id = 9;
> +} else if (atoi(x4->level) > 9) {
> +level_id = atoi(x4->level);
this should check that teres no "tail" after the integer
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
I am the wisest man alive, for I know one thing, and that is that I know
nothing. -- Socrates
signature.asc
Description: Digital signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH]lavc/x264: Improve level setting
On Tuesday 12 January 2016 01:55:30 pm Michael Niedermayer wrote:
> On Mon, Jan 11, 2016 at 10:58:55AM +0100, Carl Eugen Hoyos wrote:
> > Hi!
> >
> > I guess that attached patch fixes the additional issue in ticket #3307.
> >
> > Please comment, Carl Eugen
> >
> > libx264.c |2 ++
> > 1 file changed, 2 insertions(+)
> > cea8163693a2a78b9dc2d1929082e7c76ac542ac patchx264level.diff
> > diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
> > index 88406a3..c1e52a1 100644
> > --- a/libavcodec/libx264.c
> > +++ b/libavcodec/libx264.c
> > @@ -559,6 +559,8 @@ static av_cold int X264_init(AVCodecContext *avctx)
> >
> > if (!strcmp(x4->level, "1b")) {
> > level_id = 9;
> > +} else if (atoi(x4->level) > 9) {
Should be "> 8".
> > +level_id = atoi(x4->level);
>
> this should check that teres no "tail" after the integer
For which command line will this make a difference?
I didn't find one...
New patch attached, Carl Eugen
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 88406a3..abb6985 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -559,6 +559,8 @@ static av_cold int X264_init(AVCodecContext *avctx)
if (!strcmp(x4->level, "1b")) {
level_id = 9;
+} else if (av_strtod(x4->level, ) > 8.0 && !*tail) {
+level_id = atoi(x4->level);
} else if (strlen(x4->level) <= 3){
level_id = av_strtod(x4->level, ) * 10 + 0.5;
if (*tail)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH]lavc/x264: Improve level setting
Hi!
I guess that attached patch fixes the additional issue in ticket #3307.
Please comment, Carl Eugen
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 88406a3..c1e52a1 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -559,6 +559,8 @@ static av_cold int X264_init(AVCodecContext *avctx)
if (!strcmp(x4->level, "1b")) {
level_id = 9;
+} else if (atoi(x4->level) > 9) {
+level_id = atoi(x4->level);
} else if (strlen(x4->level) <= 3){
level_id = av_strtod(x4->level, ) * 10 + 0.5;
if (*tail)
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
