diff mbox

[FFmpeg-devel] ffmpeg: Don't offer H.264 compatibility warning for NV12 input

Message ID e4df8f35-5748-9e60-9cbe-0b2fa1bf5f3f@jkqxz.net
State New
Headers show

Commit Message

Mark Thompson June 14, 2017, 10:03 p.m. UTC
It's also 8-bit YUV 4:2:0.
---
Most visible with streams downloaded from hardware to encode properly with libx264, which will typically be NV12.


 ffmpeg.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

wm4 June 14, 2017, 11:29 p.m. UTC | #1
On Wed, 14 Jun 2017 23:03:39 +0100
Mark Thompson <sw@jkqxz.net> wrote:

> It's also 8-bit YUV 4:2:0.
> ---
> Most visible with streams downloaded from hardware to encode properly with libx264, which will typically be NV12.
> 
> 
>  ffmpeg.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ffmpeg.c b/ffmpeg.c
> index 6170bd453c..e6e8b9e119 100644
> --- a/ffmpeg.c
> +++ b/ffmpeg.c
> @@ -3345,7 +3345,8 @@ static int init_output_stream_encode(OutputStream *ost)
>              av_buffersink_get_sample_aspect_ratio(ost->filter->filter);
>          if (!strncmp(ost->enc->name, "libx264", 7) &&
>              enc_ctx->pix_fmt == AV_PIX_FMT_NONE &&
> -            av_buffersink_get_format(ost->filter->filter) != AV_PIX_FMT_YUV420P)
> +            av_buffersink_get_format(ost->filter->filter) != AV_PIX_FMT_YUV420P &&
> +            av_buffersink_get_format(ost->filter->filter) != AV_PIX_FMT_NV12)
>              av_log(NULL, AV_LOG_WARNING,
>                     "No pixel format specified, %s for H.264 encoding chosen.\n"
>                     "Use -pix_fmt yuv420p for compatibility with outdated media players.\n",

This warning shouldn't exist in the first place. It's dumb and
incorrect.
Gyan June 15, 2017, 4:12 a.m. UTC | #2
On Thu, Jun 15, 2017 at 4:59 AM, wm4 <nfxjfg@googlemail.com> wrote:

>
> >              av_log(NULL, AV_LOG_WARNING,
> >                     "No pixel format specified, %s for H.264 encoding
> chosen.\n"
> >                     "Use -pix_fmt yuv420p for compatibility with
> outdated media players.\n",
>
> This warning shouldn't exist in the first place. It's dumb and
> incorrect.
>

It could (and should) be better worded but it's needed. There's a
semi-regular influx of questions at the SE sites where users complain about
their ffmpeg output having audio but 'no video' or having a video stream
which only plays in certain players. I can only imagine that the presence
of the warning cuts down on the frequency of those queries, but also
there's likely a countervailing effect by the use of the term 'outdated'
which implies that if one has a recent version of a player, this warning
can be ignored.  When, in fact, browser-based players don't decode anything
other than yuv420p , whichever version, and same's the case for many
players on WIndows, including atleast WMP on Win7.
wm4 June 15, 2017, 9:34 a.m. UTC | #3
On Thu, 15 Jun 2017 09:42:48 +0530
Gyan <gyandoshi@gmail.com> wrote:

> On Thu, Jun 15, 2017 at 4:59 AM, wm4 <nfxjfg@googlemail.com> wrote:
> 
> >  
> > >              av_log(NULL, AV_LOG_WARNING,
> > >                     "No pixel format specified, %s for H.264 encoding  
> > chosen.\n"  
> > >                     "Use -pix_fmt yuv420p for compatibility with  
> > outdated media players.\n",
> >
> > This warning shouldn't exist in the first place. It's dumb and
> > incorrect.
> >  
> 
> It could (and should) be better worded but it's needed. There's a
> semi-regular influx of questions at the SE sites where users complain about
> their ffmpeg output having audio but 'no video' or having a video stream
> which only plays in certain players. I can only imagine that the presence
> of the warning cuts down on the frequency of those queries, but also
> there's likely a countervailing effect by the use of the term 'outdated'
> which implies that if one has a recent version of a player, this warning
> can be ignored.  When, in fact, browser-based players don't decode anything
> other than yuv420p , whichever version, and same's the case for many
> players on WIndows, including atleast WMP on Win7.

It's wrong and dumb because the pixfmt is not enough to tell whether
the encoder will produce a file with w widely compatible profile.

There is also a large number of other cases, where ffmpeg CLI would
produce files that are not widely compatible (because it uses e.g. an
obscure container/codec combination), and it wouldn't warn.

I think what you really want is either
- defaulting h264 encoding to Main or High profile only
- proper presets, that make sure the output is compatible to a certain
  class of consumers (and make them usable, unless the current
  horseshit)
Gyan June 15, 2017, 10:42 a.m. UTC | #4
On Thu, Jun 15, 2017 at 3:04 PM, wm4 <nfxjfg@googlemail.com> wrote:


> It's wrong and dumb because the pixfmt is not enough to tell whether
> the encoder will produce a file with w widely compatible profile.
>

yuv420p is not sufficient to ensure compatibility but !yuv420p is
sufficient to ensure incompatibility (with browsers and most players).


> I think what you really want is either
> - defaulting h264 encoding to Main or High profile only
>

 libx264 already defaults to High, which doesn't prevent non-yuv420p output
e.g. encoding of a PNG sequence.

Presets bring their other baggage along with them, which the user may not
want. If the user knew how to select or set presets, they would know which
pixel format is required. The primary audience for this warning is the lay
or occasional user, who isn't manually setting the profile or the pix_fmt,
and and is likely to get tripped up if the input is not yuv420p.
wm4 June 15, 2017, 11:14 a.m. UTC | #5
On Thu, 15 Jun 2017 16:12:00 +0530
Gyan <gyandoshi@gmail.com> wrote:

> On Thu, Jun 15, 2017 at 3:04 PM, wm4 <nfxjfg@googlemail.com> wrote:
> 
> 
> > It's wrong and dumb because the pixfmt is not enough to tell whether
> > the encoder will produce a file with w widely compatible profile.
> >  
> 
> yuv420p is not sufficient to ensure compatibility but !yuv420p is
> sufficient to ensure incompatibility (with browsers and most players).

That doesn't conflict with what I said.

> 
> > I think what you really want is either
> > - defaulting h264 encoding to Main or High profile only
> >  
> 
>  libx264 already defaults to High, which doesn't prevent non-yuv420p output
> e.g. encoding of a PNG sequence.

"non-yuv420p output" doesn't make ANY sense. You don't know and don't
have to care what some random third party decoders will output. Many
indeed never output yuv420p, but use nv12 and even packed or tiled
formats.

The same is true for encoder input. As the patch shows, not all
encoders are restricted to yuv420p even when encoding only to the simple
profiles.

Forcing the "High" or maybe "Main" profile is enough to ensure
compatibility. That is what profiles were designed for. Applying your
bogus "non-yuv420p output" idea on libavcodec's decoder, libavcodec
would indeed never output non-yuv420p for High profile input (unless you
use hwaccel, then it would most likely output nv12).

> Presets bring their other baggage along with them, which the user may not
> want. If the user knew how to select or set presets, they would know which
> pixel format is required. The primary audience for this warning is the lay
> or occasional user, who isn't manually setting the profile or the pix_fmt,
> and and is likely to get tripped up if the input is not yuv420p.

By default, ffmpeg CLI's design is something like "fuck the user". I'm
not sure what you are even arguing about. The situation is bad, and the
warning is a shitty insufficient hack trying to prevent damage caused
by that.
Michael Niedermayer June 15, 2017, 12:06 p.m. UTC | #6
On Thu, Jun 15, 2017 at 01:14:34PM +0200, wm4 wrote:
[...]
> By default, ffmpeg CLI's design is something like "fuck the user". I'm

This is not true, nor are statments like this useful.
Whatever problem there is, it is not and never was FFmpegs design to
fuck the user. Quite the opposite ...

If problems exist they should be fixed. If they cannot be fixed they
should be at least documented on our issue tracker.

Without this, such rants are off topic as they cannot serve to
improve FFmpeg. Noone can improve ffmpegs CLI from a incorrect
statement of the problem.
There may be issues in the CLI, but the issue is not that its
designed to "fuck the user"

Also id love to see other members of the mailing list, there are over
a thousand IIRC. Help a bit with keeping the content and tone of the
ffmpeg mailing lists within the purpose of the lists.
Which is ultimatly to improve FFmpeg and help its users and developers.

thanks

> not sure what you are even arguing about. The situation is bad, and the
> warning is a shitty insufficient hack trying to prevent damage caused
> by that.
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Paul B Mahol June 15, 2017, 12:12 p.m. UTC | #7
On 6/15/17, Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Thu, Jun 15, 2017 at 01:14:34PM +0200, wm4 wrote:
> [...]
>> By default, ffmpeg CLI's design is something like "fuck the user". I'm
>
> This is not true, nor are statments like this useful.
> Whatever problem there is, it is not and never was FFmpegs design to
> fuck the user. Quite the opposite ...
>
> If problems exist they should be fixed. If they cannot be fixed they
> should be at least documented on our issue tracker.
>
> Without this, such rants are off topic as they cannot serve to
> improve FFmpeg. Noone can improve ffmpegs CLI from a incorrect
> statement of the problem.
> There may be issues in the CLI, but the issue is not that its
> designed to "fuck the user"
>
> Also id love to see other members of the mailing list, there are over
> a thousand IIRC. Help a bit with keeping the content and tone of the
> ffmpeg mailing lists within the purpose of the lists.
> Which is ultimatly to improve FFmpeg and help its users and developers.

We do not have supreme leader any more.
Gyan June 15, 2017, 12:47 p.m. UTC | #8
On Thu, Jun 15, 2017 at 4:44 PM, wm4 <nfxjfg@googlemail.com> wrote:


> Forcing the "High" or maybe "Main" profile is enough to ensure
> compatibility.
>

Result of encoding a PNG sequence, forcing High profile, but without
-pix_fmt

-----------------
No pixel format specified, yuv444p for H.264 encoding chosen.
Use -pix_fmt yuv420p for compatibility with outdated media players.
x264 [error]: high profile doesn't support 4:4:4
[libx264 @ 0000000002b02b40] Error setting profile high.
[libx264 @ 0000000002b02b40] Possible profiles: baseline main high high10
high422 high444
Error initializing output stream 0:0 -- Error while opening encoder for
output stream #0:0 - maybe incorrect parameters such as bit_rate, rate,
width or height
Conversion failed!
-----------------
wm4 June 15, 2017, 1:02 p.m. UTC | #9
On Thu, 15 Jun 2017 18:17:37 +0530
Gyan <gyandoshi@gmail.com> wrote:

> On Thu, Jun 15, 2017 at 4:44 PM, wm4 <nfxjfg@googlemail.com> wrote:
> 
> 
> > Forcing the "High" or maybe "Main" profile is enough to ensure
> > compatibility.
> >  
> 
> Result of encoding a PNG sequence, forcing High profile, but without
> -pix_fmt
> 
> -----------------
> No pixel format specified, yuv444p for H.264 encoding chosen.
> Use -pix_fmt yuv420p for compatibility with outdated media players.
> x264 [error]: high profile doesn't support 4:4:4
> [libx264 @ 0000000002b02b40] Error setting profile high.
> [libx264 @ 0000000002b02b40] Possible profiles: baseline main high high10
> high422 high444
> Error initializing output stream 0:0 -- Error while opening encoder for
> output stream #0:0 - maybe incorrect parameters such as bit_rate, rate,
> width or height
> Conversion failed!
> -----------------

So that's fine and the message is not needed. Where is the problem?
Other than ffmpeg not doing the obvious thing, and converting to
yuv420p (no, the discussed message has absolutely nothing to do with
this).

I'm fairly sure this is the fault of the libavcodec encoding API having
no format negotiation API, though. Didn't dig into it further.
Michael Niedermayer June 15, 2017, 2:22 p.m. UTC | #10
On Thu, Jun 15, 2017 at 02:12:09PM +0200, Paul B Mahol wrote:
> On 6/15/17, Michael Niedermayer <michael@niedermayer.cc> wrote:
> > On Thu, Jun 15, 2017 at 01:14:34PM +0200, wm4 wrote:
> > [...]
> >> By default, ffmpeg CLI's design is something like "fuck the user". I'm
> >
> > This is not true, nor are statments like this useful.
> > Whatever problem there is, it is not and never was FFmpegs design to
> > fuck the user. Quite the opposite ...
> >
> > If problems exist they should be fixed. If they cannot be fixed they
> > should be at least documented on our issue tracker.
> >
> > Without this, such rants are off topic as they cannot serve to
> > improve FFmpeg. Noone can improve ffmpegs CLI from a incorrect
> > statement of the problem.
> > There may be issues in the CLI, but the issue is not that its
> > designed to "fuck the user"
> >
> > Also id love to see other members of the mailing list, there are over
> > a thousand IIRC. Help a bit with keeping the content and tone of the
> > ffmpeg mailing lists within the purpose of the lists.
> > Which is ultimatly to improve FFmpeg and help its users and developers.
> 
> We do not have supreme leader any more.

no, but that doesnt mean we can make false statments about other
people or their work.

Theres the moral wrong

Theres the technical wrong, of not correctly stating the problem and
that potentially affecting work on the problem

Theres the social wrong. That is people who spend significant time on
the CLI becoming offended and alienated by such statments.

Theres the legal wrong, there are many people who worked on the CLI
at some point. Anyone of them can probably sue us for defamation,
but IANAL. I dont think that would help FFmpegs / our public image.


A big part of the current CLI has btw been implemented by Libav
developers, and merged into FFmpeg, other parts are very old code
from times when FFmpeg was much simpler.


[...]
wm4 June 15, 2017, 2:43 p.m. UTC | #11
On Thu, 15 Jun 2017 16:22:34 +0200
Michael Niedermayer <michael@niedermayer.cc> wrote:

> no, but that doesnt mean we can make false statments about other
> people or their work.
> 
> Theres the moral wrong
> 
> Theres the technical wrong, of not correctly stating the problem and
> that potentially affecting work on the problem
> 
> Theres the social wrong. That is people who spend significant time on
> the CLI becoming offended and alienated by such statments.
> 
> Theres the legal wrong, there are many people who worked on the CLI
> at some point. Anyone of them can probably sue us for defamation,
> but IANAL. I dont think that would help FFmpegs / our public image.

I think you're blowing this out of proportion, as usual (I guess I
should sue myself because I've worked on ffmpeg.c too?).

But my particular problem here is that some people just can't admit that
certain parts of the code are just bad, and that they actively fight
cleaning it up. Fuck that, I don't see any reason to hold back under
these circumstances.

ffmpeg.c in particular is a vile POS. Whenever there is a problem, I
find half a dozen more problems, no clear way to fix it, and general
misery. If I want to try to find out how to do something specific with
ffmpeg.c, its help output is utterly useless and confusing, and I end
up reading the code. I guess it produces more stackoverflow traffic and
more need for ffmpeg specialists (especially when there's payment), so
I guess it has something good too.

> A big part of the current CLI has btw been implemented by Libav
> developers, and merged into FFmpeg, other parts are very old code
> from times when FFmpeg was much simpler.

Who mentioned Libav?
James Almer June 15, 2017, 3:10 p.m. UTC | #12
On 6/15/2017 11:43 AM, wm4 wrote:
> On Thu, 15 Jun 2017 16:22:34 +0200
> Michael Niedermayer <michael@niedermayer.cc> wrote:
> 
>> no, but that doesnt mean we can make false statments about other
>> people or their work.
>>
>> Theres the moral wrong
>>
>> Theres the technical wrong, of not correctly stating the problem and
>> that potentially affecting work on the problem
>>
>> Theres the social wrong. That is people who spend significant time on
>> the CLI becoming offended and alienated by such statments.
>>
>> Theres the legal wrong, there are many people who worked on the CLI
>> at some point. Anyone of them can probably sue us for defamation,
>> but IANAL. I dont think that would help FFmpegs / our public image.
> 
> I think you're blowing this out of proportion, as usual (I guess I
> should sue myself because I've worked on ffmpeg.c too?).
> 
> But my particular problem here is that some people just can't admit that
> certain parts of the code are just bad, and that they actively fight
> cleaning it up. Fuck that, I don't see any reason to hold back under
> these circumstances.
> 
> ffmpeg.c in particular is a vile POS. Whenever there is a problem, I
> find half a dozen more problems, no clear way to fix it, and general
> misery. If I want to try to find out how to do something specific with
> ffmpeg.c, its help output is utterly useless and confusing, and I end
> up reading the code. I guess it produces more stackoverflow traffic and
> more need for ffmpeg specialists (especially when there's payment), so
> I guess it has something good too.

Why did a patch to remove a warning from certain cases ended up in shit
flinging I'll never know. But I'd very much like it if it stopped here.

> 
>> A big part of the current CLI has btw been implemented by Libav
>> developers, and merged into FFmpeg, other parts are very old code
>> from times when FFmpeg was much simpler.
> 
> Who mentioned Libav?
wm4 June 15, 2017, 4:49 p.m. UTC | #13
On Thu, 15 Jun 2017 12:10:27 -0300
James Almer <jamrial@gmail.com> wrote:

> On 6/15/2017 11:43 AM, wm4 wrote:
> > On Thu, 15 Jun 2017 16:22:34 +0200
> > Michael Niedermayer <michael@niedermayer.cc> wrote:
> >   
> >> no, but that doesnt mean we can make false statments about other
> >> people or their work.
> >>
> >> Theres the moral wrong
> >>
> >> Theres the technical wrong, of not correctly stating the problem and
> >> that potentially affecting work on the problem
> >>
> >> Theres the social wrong. That is people who spend significant time on
> >> the CLI becoming offended and alienated by such statments.
> >>
> >> Theres the legal wrong, there are many people who worked on the CLI
> >> at some point. Anyone of them can probably sue us for defamation,
> >> but IANAL. I dont think that would help FFmpegs / our public image.  
> > 
> > I think you're blowing this out of proportion, as usual (I guess I
> > should sue myself because I've worked on ffmpeg.c too?).
> > 
> > But my particular problem here is that some people just can't admit that
> > certain parts of the code are just bad, and that they actively fight
> > cleaning it up. Fuck that, I don't see any reason to hold back under
> > these circumstances.
> > 
> > ffmpeg.c in particular is a vile POS. Whenever there is a problem, I
> > find half a dozen more problems, no clear way to fix it, and general
> > misery. If I want to try to find out how to do something specific with
> > ffmpeg.c, its help output is utterly useless and confusing, and I end
> > up reading the code. I guess it produces more stackoverflow traffic and
> > more need for ffmpeg specialists (especially when there's payment), so
> > I guess it has something good too.  
> 
> Why did a patch to remove a warning from certain cases ended up in shit
> flinging I'll never know. But I'd very much like it if it stopped here.

You're right, I'll propose a patch to remove this message.
diff mbox

Patch

diff --git a/ffmpeg.c b/ffmpeg.c
index 6170bd453c..e6e8b9e119 100644
--- a/ffmpeg.c
+++ b/ffmpeg.c
@@ -3345,7 +3345,8 @@  static int init_output_stream_encode(OutputStream *ost)
             av_buffersink_get_sample_aspect_ratio(ost->filter->filter);
         if (!strncmp(ost->enc->name, "libx264", 7) &&
             enc_ctx->pix_fmt == AV_PIX_FMT_NONE &&
-            av_buffersink_get_format(ost->filter->filter) != AV_PIX_FMT_YUV420P)
+            av_buffersink_get_format(ost->filter->filter) != AV_PIX_FMT_YUV420P &&
+            av_buffersink_get_format(ost->filter->filter) != AV_PIX_FMT_NV12)
             av_log(NULL, AV_LOG_WARNING,
                    "No pixel format specified, %s for H.264 encoding chosen.\n"
                    "Use -pix_fmt yuv420p for compatibility with outdated media players.\n",