Re: [FFmpeg-devel] How to correctly handle Matroska timestamps

2021-08-07 Thread Andreas Rheinhardt
MattKC:
> ==Summary==
> 
> I've been implementing ffmpeg/libav into an application, and have run
> into timing-related issues when playing back Matroska/MKV videos created
> by my application in VLC. I tried asking about this on the libav-user
> mailing list and received no response so I thought I'd try here. While
> it's true that there may be issues with VLC's implementation too, I've
> noticed specifically that videos generated by FFmpeg do not cause these
> issues, while my application and the examples in "doc/examples" (which
> my code is modeled from) do, which makes me think this code is missing
> something crucial.
> 
> ==Symptoms==
> 
> During playback, VLC's playback time will appear to stay frozen at 0:00
> while the video and audio plays normally. It will then jump only in
> increments of a few seconds (0:03, then 0:07, then 0:11, etc.) This
> behavior occurs with videos produced both by my application and by the
> FFmpeg examples programs "transcoding.c" and "remuxing.c".
> 
> The fact that this occurs with "remuxing.c" is particularly interesting
> since that would seem to imply it's an issue with the format/container
> writing specifically. If "remuxing.c" is given a correct MKV file (i.e.
> one that does not exhibit this issue) produced by FFmpeg as its input,
> and outputs to a second MKV file, that second MKV will demonstrate this
> issue even though the first one didn't, indicating that something is
> breaking during the remuxing process despite no decoding/encoding
> occurring.
> 
> The worst demonstration of this issue that I've run into is if my
> application writes any audio packets before writing the first video
> packet (with "av_interleaved_write_frame"), the first few seconds of
> video (until that first 0:03) will be considered completely blank by
> VLC. The fact that it will pick up after 0:03, the same time the timer
> will make its first update on other MKV files, leads me to believe it's
> the exact same root cause, just a different presentation of it.
> 
> This issue only occurs on MKV files. MP4/MOV appear to have no issues
> whatsoever.
> 
> ==Conclusion==
> 
> Considering FFmpeg produces correct MKV files, I don't believe this to
> be a bug in libav (though it could be considered a bug in the examples).
> I'm assuming the examples and my program are missing something or doing
> something incorrectly.
> 
> My code can be seen here, however the examples will almost certainly be
> easier to read and test:
> https://github.com/olive-editor/olive/blob/master/app/codec/ffmpeg/ffmpegencoder.cpp
> 
> 
> Thanks in advance for any help, I'm a little confused about this whole
> issue.
> 
If it happens with our examples, too (as you said), then you should open
an issue on trac (for the examples). Be sure to add samples that allow
to reproduce it.
It would probably be enlightening to use mkvinfo to look at the
generated files. The files are probably badly interleaved (your
description makes me suspect that the generated files have lots of audio
packets at the beginning before the first video packet).
(A quick look showed that the transcoding example does not use
pkt_timebase for decoding, as does your code; furthermore, your code
sets the duration of subtitle packets wrong, as the end_display_time is
in a different timebase than the subtitle's pts.)

- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] fftools/cmdutils: don't print build configuration by default

2021-08-07 Thread Gyan Doshi




On 2021-08-06 11:34 pm, James Almer wrote:

From: Matthieu Patou 

Suggested-by: ffm...@fb.com
Signed-off-by: James Almer 
---
  fftools/cmdutils.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fftools/cmdutils.c b/fftools/cmdutils.c
index 912e881174..fc58277df7 100644
--- a/fftools/cmdutils.c
+++ b/fftools/cmdutils.c
@@ -1160,7 +1160,8 @@ static void print_program_info(int flags, int level)
  av_log(NULL, level, "\n");
  av_log(NULL, level, "%sbuilt with %s\n", indent, CC_IDENT);
  
-av_log(NULL, level, "%sconfiguration: " FFMPEG_CONFIGURATION "\n", indent);

+if (flags & SHOW_CONFIG)
+av_log(NULL, level, "%sconfiguration: " FFMPEG_CONFIGURATION "\n", 
indent);


I like it. If most users pasted the full log on their own initiative in 
bug reports or Q then this would have been a negative but they don't.


I suggest to add an else clause that prints something like "Add 
-buildconf to view configuration".


Regards,
Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] How to correctly handle Matroska timestamps

2021-08-07 Thread MattKC

==Summary==

I've been implementing ffmpeg/libav into an application, and have run 
into timing-related issues when playing back Matroska/MKV videos created 
by my application in VLC. I tried asking about this on the libav-user 
mailing list and received no response so I thought I'd try here. While 
it's true that there may be issues with VLC's implementation too, I've 
noticed specifically that videos generated by FFmpeg do not cause these 
issues, while my application and the examples in "doc/examples" (which 
my code is modeled from) do, which makes me think this code is missing 
something crucial.


==Symptoms==

During playback, VLC's playback time will appear to stay frozen at 0:00 
while the video and audio plays normally. It will then jump only in 
increments of a few seconds (0:03, then 0:07, then 0:11, etc.) This 
behavior occurs with videos produced both by my application and by the 
FFmpeg examples programs "transcoding.c" and "remuxing.c".


The fact that this occurs with "remuxing.c" is particularly interesting 
since that would seem to imply it's an issue with the format/container 
writing specifically. If "remuxing.c" is given a correct MKV file (i.e. 
one that does not exhibit this issue) produced by FFmpeg as its input, 
and outputs to a second MKV file, that second MKV will demonstrate this 
issue even though the first one didn't, indicating that something is 
breaking during the remuxing process despite no decoding/encoding occurring.


The worst demonstration of this issue that I've run into is if my 
application writes any audio packets before writing the first video 
packet (with "av_interleaved_write_frame"), the first few seconds of 
video (until that first 0:03) will be considered completely blank by 
VLC. The fact that it will pick up after 0:03, the same time the timer 
will make its first update on other MKV files, leads me to believe it's 
the exact same root cause, just a different presentation of it.


This issue only occurs on MKV files. MP4/MOV appear to have no issues 
whatsoever.


==Conclusion==

Considering FFmpeg produces correct MKV files, I don't believe this to 
be a bug in libav (though it could be considered a bug in the examples). 
I'm assuming the examples and my program are missing something or doing 
something incorrectly.


My code can be seen here, however the examples will almost certainly be 
easier to read and test: 
https://github.com/olive-editor/olive/blob/master/app/codec/ffmpeg/ffmpegencoder.cpp


Thanks in advance for any help, I'm a little confused about this whole 
issue.


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v3] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread Soft Works
Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a check for 
value_len > UINT16_MAX.
As a consequence, attached images of sizes larger than UINT16_MAX could no 
longer be read.

Signed-off-by: softworkz 
---
 libavformat/asfdec_f.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index ff6ddfb967..1be21bdf82 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -614,7 +614,7 @@ static int asf_read_metadata(AVFormatContext *s, int64_t 
size)
 value_type = avio_rl16(pb); /* value_type */
 value_len  = avio_rl32(pb);
 
-if (value_len < 0 || value_len > UINT16_MAX)
+if (value_len < 0 || value_len >= (INT_MAX - LEN) / 2)
 return AVERROR_INVALIDDATA;
 
 name_len_utf8 = 2*name_len_utf16 + 1;
-- 
2.28.0.windows.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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread Soft Works



> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> James Almer
> Sent: Sunday, 8 August 2021 03:19
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression
> bug when reading image attachments
> 
> On 8/7/2021 10:00 PM, Soft Works wrote:
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
> >> James Almer
> >> Sent: Sunday, 8 August 2021 02:47
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix
> >> regression bug when reading image attachments
> >>
> >> On 8/7/2021 9:38 PM, Soft Works wrote:
>  -Original Message-
>  From: ffmpeg-devel  On Behalf
> Of
>  Carl Eugen Hoyos
>  Sent: Sunday, 8 August 2021 01:58
>  To: FFmpeg development discussions and patches   de...@ffmpeg.org>
>  Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix
>  regression bug when reading image attachments
> 
>  Am So., 8. Aug. 2021 um 01:53 Uhr schrieb Soft Works
>  :
> >
> > Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a
>  check for value_len > UINT16_MAX.
> > As a consequence, attached images of sizes larger than UINT16_MAX
> > could
>  no longer be read.
> >
> > Signed-off-by: softworkz 
> > ---
> > v2: Fix without changing variable type  libavformat/asfdec_f.c | 2 +-
> >1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c index
> > ff6ddfb967..b9f3918495 100644
> > --- a/libavformat/asfdec_f.c
> > +++ b/libavformat/asfdec_f.c
> > @@ -614,7 +614,7 @@ static int
> asf_read_metadata(AVFormatContext
> >> *s,
>  int64_t size)
> >value_type = avio_rl16(pb); /* value_type */
> >value_len  = avio_rl32(pb);
> >
> > -if (value_len < 0 || value_len > UINT16_MAX)
> > +if (value_len < 0)
> >return AVERROR_INVALIDDATA;
> 
>  I may misread the code but it appears to me that an assert can be
>  triggered now, no?
> >>>
> >>> Right, for these 11 discrete size values only, though:
> >>>
> >>> 2147483647, 2147483646, 2147483645, 2147483644, 2147483643,
> >>> 2147483642, 2147483641, 2147483640, 2147483639, 2147483638
> >>> 2147483636
> >>>
> >>> A chance of 11 in 4 Billions :-)
> >>
> >> len < (INT_MAX - LEN) / 2 is more than half the range of an int.
> >
> > I've been one step ahead in calculation:
> >
> > av_malloc takes size_t and on 32bit platforms, this is uint32, which
> > means that it can take a maximum of 4.294.967.295.
> > (4.294.967.295 - 22) / 2 = 2.147.483.636  (+ 11 makes INT32_MAX)
> 
> The assert right now checks strictly for INT_MAX, not UINT_MAX. If len is >=
> 1073741812, it will trigger. Whatever av_malloc() could handle afterwards
> does not play a part in that.
> 
> >
> >>
> >>>
> >>> TBH, I don't understand this assert. When the attachment is too
> >>> large to handle, we should rather log a warning and goto finish instead
> IMO.
> >>
> >> The assert is to ensure get_tag() is not called with out of range
> >> values, so the relevant checks should happen before the call.
> >
> > I'm not sure whether I'd agree with that. It shouldn't be the caller
> > needing to know what the function can handle and what it can't handle.
> >
> > The INT32/2 is not only wrong, it also does _only_ apply to reading
> > unicode. For ASCII and byte arrays, there is no need for this restriction.
> > And on 64bit platforms, there's no need for this that restriction at all.
> 
> If you want to change this and allow values higher than (INT_MAX - LEN) / 2
> or even replace the assert with a normal check then feel free to, but your
> patch as is will trigger this assert on more than half the possible values of 
> len.

When I do this:

if (value_len < 0 || value_len >= (INT_MAX - LEN) / 2)
return AVERROR_INVALIDDATA;

Shouldn't we return a different error code? Actually this is not about invalid
data but about a limitation in the implementation..

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread Soft Works



> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> James Almer
> Sent: Sunday, 8 August 2021 03:19
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression
> bug when reading image attachments
> 
> On 8/7/2021 10:00 PM, Soft Works wrote:
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
> >> James Almer
> >> Sent: Sunday, 8 August 2021 02:47
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix
> >> regression bug when reading image attachments
> >>
> >> On 8/7/2021 9:38 PM, Soft Works wrote:
>  -Original Message-
>  From: ffmpeg-devel  On Behalf
> Of
>  Carl Eugen Hoyos
>  Sent: Sunday, 8 August 2021 01:58
>  To: FFmpeg development discussions and patches   de...@ffmpeg.org>
>  Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix
>  regression bug when reading image attachments
> 
>  Am So., 8. Aug. 2021 um 01:53 Uhr schrieb Soft Works
>  :
> >
> > Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a
>  check for value_len > UINT16_MAX.
> > As a consequence, attached images of sizes larger than UINT16_MAX
> > could
>  no longer be read.
> >
> > Signed-off-by: softworkz 
> > ---
> > v2: Fix without changing variable type  libavformat/asfdec_f.c | 2 +-
> >1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c index
> > ff6ddfb967..b9f3918495 100644
> > --- a/libavformat/asfdec_f.c
> > +++ b/libavformat/asfdec_f.c
> > @@ -614,7 +614,7 @@ static int
> asf_read_metadata(AVFormatContext
> >> *s,
>  int64_t size)
> >value_type = avio_rl16(pb); /* value_type */
> >value_len  = avio_rl32(pb);
> >
> > -if (value_len < 0 || value_len > UINT16_MAX)
> > +if (value_len < 0)
> >return AVERROR_INVALIDDATA;
> 
>  I may misread the code but it appears to me that an assert can be
>  triggered now, no?
> >>>
> >>> Right, for these 11 discrete size values only, though:
> >>>
> >>> 2147483647, 2147483646, 2147483645, 2147483644, 2147483643,
> >>> 2147483642, 2147483641, 2147483640, 2147483639, 2147483638
> >>> 2147483636
> >>>
> >>> A chance of 11 in 4 Billions :-)
> >>
> >> len < (INT_MAX - LEN) / 2 is more than half the range of an int.
> >
> > I've been one step ahead in calculation:
> >
> > av_malloc takes size_t and on 32bit platforms, this is uint32, which
> > means that it can take a maximum of 4.294.967.295.
> > (4.294.967.295 - 22) / 2 = 2.147.483.636  (+ 11 makes INT32_MAX)
> 
> The assert right now checks strictly for INT_MAX, not UINT_MAX. If len is >=
> 1073741812, it will trigger. Whatever av_malloc() could handle afterwards
> does not play a part in that.
> 
> >
> >>
> >>>
> >>> TBH, I don't understand this assert. When the attachment is too
> >>> large to handle, we should rather log a warning and goto finish instead
> IMO.
> >>
> >> The assert is to ensure get_tag() is not called with out of range
> >> values, so the relevant checks should happen before the call.
> >
> > I'm not sure whether I'd agree with that. It shouldn't be the caller
> > needing to know what the function can handle and what it can't handle.
> >
> > The INT32/2 is not only wrong, it also does _only_ apply to reading
> > unicode. For ASCII and byte arrays, there is no need for this restriction.
> > And on 64bit platforms, there's no need for this that restriction at all.
> 
> If you want to change this and allow values higher than (INT_MAX - LEN) / 2
> or even replace the assert with a normal check then feel free to, but your
> patch as is will trigger this assert on more than half the possible values of 
> len.

Yea that's true of course. Honestly, I don't know what to do. There more I
look around the more flawed do things appear. Just look at the type2_size
parameter of get_tag and all callers supply values like 32 and 16, but actually
that parameter should not exist at all: it is only used for the types bool, 
qword,
dword and word, and for these cases, the length is defined by the spec and
should not be specified by callers.

Or look at the byte_array type, where it is allocating the double size of the
byte array without actually using it.

Adding that value of 22 is actually not needed at all. It's just developer 
laziness
for having at least 22 bytes in case that the give length is zero.

A little bit frightening all this. :-) 
I guess I stay on the simple side for now..
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread James Almer

On 8/7/2021 10:00 PM, Soft Works wrote:

-Original Message-
From: ffmpeg-devel  On Behalf Of
James Almer
Sent: Sunday, 8 August 2021 02:47
To: ffmpeg-devel@ffmpeg.org
Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression
bug when reading image attachments

On 8/7/2021 9:38 PM, Soft Works wrote:

-Original Message-
From: ffmpeg-devel  On Behalf Of
Carl Eugen Hoyos
Sent: Sunday, 8 August 2021 01:58
To: FFmpeg development discussions and patches 
Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix
regression bug when reading image attachments

Am So., 8. Aug. 2021 um 01:53 Uhr schrieb Soft Works
:


Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a

check for value_len > UINT16_MAX.

As a consequence, attached images of sizes larger than UINT16_MAX
could

no longer be read.


Signed-off-by: softworkz 
---
v2: Fix without changing variable type  libavformat/asfdec_f.c | 2 +-
   1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c index
ff6ddfb967..b9f3918495 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -614,7 +614,7 @@ static int asf_read_metadata(AVFormatContext

*s,

int64_t size)

   value_type = avio_rl16(pb); /* value_type */
   value_len  = avio_rl32(pb);

-if (value_len < 0 || value_len > UINT16_MAX)
+if (value_len < 0)
   return AVERROR_INVALIDDATA;


I may misread the code but it appears to me that an assert can be
triggered now, no?


Right, for these 11 discrete size values only, though:

2147483647, 2147483646, 2147483645, 2147483644, 2147483643,
2147483642, 2147483641, 2147483640, 2147483639, 2147483638
2147483636

A chance of 11 in 4 Billions :-)


len < (INT_MAX - LEN) / 2 is more than half the range of an int.


I've been one step ahead in calculation:

av_malloc takes size_t and on 32bit platforms, this is uint32, which means
that it can take a maximum of 4.294.967.295.
(4.294.967.295 - 22) / 2 = 2.147.483.636  (+ 11 makes INT32_MAX)


The assert right now checks strictly for INT_MAX, not UINT_MAX. If len 
is >= 1073741812, it will trigger. Whatever av_malloc() could handle 
afterwards does not play a part in that.








TBH, I don't understand this assert. When the attachment is too large
to handle, we should rather log a warning and goto finish instead IMO.


The assert is to ensure get_tag() is not called with out of range values, so the
relevant checks should happen before the call.


I'm not sure whether I'd agree with that. It shouldn't be the caller
needing to know what the function can handle and what it can't handle.

The INT32/2 is not only wrong, it also does _only_ apply to reading
unicode. For ASCII and byte arrays, there is no need for this restriction.
And on 64bit platforms, there's no need for this that restriction at all.


If you want to change this and allow values higher than (INT_MAX - LEN) 
/ 2 or even replace the assert with a normal check then feel free to, 
but your patch as is will trigger this assert on more than half the 
possible values of len.




softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread Soft Works



> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Carl Eugen Hoyos
> Sent: Sunday, 8 August 2021 03:03
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression
> bug when reading image attachments
> 
> Am So., 8. Aug. 2021 um 03:00 Uhr schrieb Soft Works
> :
> >
> > > -Original Message-
> > > From: ffmpeg-devel  On Behalf Of
> > > James Almer
> > > Sent: Sunday, 8 August 2021 02:47
> > > To: ffmpeg-devel@ffmpeg.org
> > > Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix
> > > regression bug when reading image attachments
> > >
> > > On 8/7/2021 9:38 PM, Soft Works wrote:
> > > >> -Original Message-
> > > >> From: ffmpeg-devel  On Behalf
> Of
> > > >> Carl Eugen Hoyos
> > > >> Sent: Sunday, 8 August 2021 01:58
> > > >> To: FFmpeg development discussions and patches  > > >> de...@ffmpeg.org>
> > > >> Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix
> > > >> regression bug when reading image attachments
> > > >>
> > > >> Am So., 8. Aug. 2021 um 01:53 Uhr schrieb Soft Works
> > > >> :
> > > >>>
> > > >>> Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced
> a
> > > >> check for value_len > UINT16_MAX.
> > > >>> As a consequence, attached images of sizes larger than
> > > >>> UINT16_MAX could
> > > >> no longer be read.
> > > >>>
> > > >>> Signed-off-by: softworkz 
> > > >>> ---
> > > >>> v2: Fix without changing variable type  libavformat/asfdec_f.c | 2 +-
> > > >>>   1 file changed, 1 insertion(+), 1 deletion(-)
> > > >>>
> > > >>> diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
> > > >>> index
> > > >>> ff6ddfb967..b9f3918495 100644
> > > >>> --- a/libavformat/asfdec_f.c
> > > >>> +++ b/libavformat/asfdec_f.c
> > > >>> @@ -614,7 +614,7 @@ static int
> asf_read_metadata(AVFormatContext
> > > *s,
> > > >> int64_t size)
> > > >>>   value_type = avio_rl16(pb); /* value_type */
> > > >>>   value_len  = avio_rl32(pb);
> > > >>>
> > > >>> -if (value_len < 0 || value_len > UINT16_MAX)
> > > >>> +if (value_len < 0)
> > > >>>   return AVERROR_INVALIDDATA;
> > > >>
> > > >> I may misread the code but it appears to me that an assert can be
> > > >> triggered now, no?
> > > >
> > > > Right, for these 11 discrete size values only, though:
> > > >
> > > > 2147483647, 2147483646, 2147483645, 2147483644, 2147483643,
> > > > 2147483642, 2147483641, 2147483640, 2147483639, 2147483638
> > > > 2147483636
> > > >
> > > > A chance of 11 in 4 Billions :-)
> > >
> > > len < (INT_MAX - LEN) / 2 is more than half the range of an int.
> >
> > I've been one step ahead in calculation:
> >
> > av_malloc takes size_t and on 32bit platforms, this is uint32, which
> > means that it can take a maximum of 4.294.967.295.
> > (4.294.967.295 - 22) / 2 = 2.147.483.636  (+ 11 makes INT32_MAX)
> >
> > >
> > > >
> > > > TBH, I don't understand this assert. When the attachment is too
> > > > large to handle, we should rather log a warning and goto finish instead
> IMO.
> > >
> > > The assert is to ensure get_tag() is not called with out of range
> > > values, so the relevant checks should happen before the call.
> >
> > I'm not sure whether I'd agree with that. It shouldn't be the caller
> > needing to know what the function can handle and what it can't handle.
> 
> > The INT32/2 is not only wrong, it also does _only_ apply to reading
> > unicode. For ASCII and byte arrays, there is no need for this restriction.
> > And on 64bit platforms, there's no need for this that restriction at all.
> 
> There is:
> An allocation >32bit for an attachment in an asf file is always wrong.

Yes, that's UINT32_MAX, but  UINT32_MAX * 2 + 22 can only be allocated
on 64bit platforms. See

value = av_malloc(2 * len + LEN);

in get_tag



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread Carl Eugen Hoyos
Am So., 8. Aug. 2021 um 03:00 Uhr schrieb Soft Works :
>
> > -Original Message-
> > From: ffmpeg-devel  On Behalf Of
> > James Almer
> > Sent: Sunday, 8 August 2021 02:47
> > To: ffmpeg-devel@ffmpeg.org
> > Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression
> > bug when reading image attachments
> >
> > On 8/7/2021 9:38 PM, Soft Works wrote:
> > >> -Original Message-
> > >> From: ffmpeg-devel  On Behalf Of
> > >> Carl Eugen Hoyos
> > >> Sent: Sunday, 8 August 2021 01:58
> > >> To: FFmpeg development discussions and patches  > >> de...@ffmpeg.org>
> > >> Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix
> > >> regression bug when reading image attachments
> > >>
> > >> Am So., 8. Aug. 2021 um 01:53 Uhr schrieb Soft Works
> > >> :
> > >>>
> > >>> Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a
> > >> check for value_len > UINT16_MAX.
> > >>> As a consequence, attached images of sizes larger than UINT16_MAX
> > >>> could
> > >> no longer be read.
> > >>>
> > >>> Signed-off-by: softworkz 
> > >>> ---
> > >>> v2: Fix without changing variable type  libavformat/asfdec_f.c | 2 +-
> > >>>   1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c index
> > >>> ff6ddfb967..b9f3918495 100644
> > >>> --- a/libavformat/asfdec_f.c
> > >>> +++ b/libavformat/asfdec_f.c
> > >>> @@ -614,7 +614,7 @@ static int asf_read_metadata(AVFormatContext
> > *s,
> > >> int64_t size)
> > >>>   value_type = avio_rl16(pb); /* value_type */
> > >>>   value_len  = avio_rl32(pb);
> > >>>
> > >>> -if (value_len < 0 || value_len > UINT16_MAX)
> > >>> +if (value_len < 0)
> > >>>   return AVERROR_INVALIDDATA;
> > >>
> > >> I may misread the code but it appears to me that an assert can be
> > >> triggered now, no?
> > >
> > > Right, for these 11 discrete size values only, though:
> > >
> > > 2147483647, 2147483646, 2147483645, 2147483644, 2147483643,
> > > 2147483642, 2147483641, 2147483640, 2147483639, 2147483638
> > > 2147483636
> > >
> > > A chance of 11 in 4 Billions :-)
> >
> > len < (INT_MAX - LEN) / 2 is more than half the range of an int.
>
> I've been one step ahead in calculation:
>
> av_malloc takes size_t and on 32bit platforms, this is uint32, which means
> that it can take a maximum of 4.294.967.295.
> (4.294.967.295 - 22) / 2 = 2.147.483.636  (+ 11 makes INT32_MAX)
>
> >
> > >
> > > TBH, I don't understand this assert. When the attachment is too large
> > > to handle, we should rather log a warning and goto finish instead IMO.
> >
> > The assert is to ensure get_tag() is not called with out of range values, 
> > so the
> > relevant checks should happen before the call.
>
> I'm not sure whether I'd agree with that. It shouldn't be the caller
> needing to know what the function can handle and what it can't handle.

> The INT32/2 is not only wrong, it also does _only_ apply to reading
> unicode. For ASCII and byte arrays, there is no need for this restriction.
> And on 64bit platforms, there's no need for this that restriction at all.

There is:
An allocation >32bit for an attachment in an asf file is always wrong.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread Soft Works
> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> James Almer
> Sent: Sunday, 8 August 2021 02:47
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression
> bug when reading image attachments
> 
> On 8/7/2021 9:38 PM, Soft Works wrote:
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
> >> Carl Eugen Hoyos
> >> Sent: Sunday, 8 August 2021 01:58
> >> To: FFmpeg development discussions and patches  >> de...@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix
> >> regression bug when reading image attachments
> >>
> >> Am So., 8. Aug. 2021 um 01:53 Uhr schrieb Soft Works
> >> :
> >>>
> >>> Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a
> >> check for value_len > UINT16_MAX.
> >>> As a consequence, attached images of sizes larger than UINT16_MAX
> >>> could
> >> no longer be read.
> >>>
> >>> Signed-off-by: softworkz 
> >>> ---
> >>> v2: Fix without changing variable type  libavformat/asfdec_f.c | 2 +-
> >>>   1 file changed, 1 insertion(+), 1 deletion(-)
> >>>
> >>> diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c index
> >>> ff6ddfb967..b9f3918495 100644
> >>> --- a/libavformat/asfdec_f.c
> >>> +++ b/libavformat/asfdec_f.c
> >>> @@ -614,7 +614,7 @@ static int asf_read_metadata(AVFormatContext
> *s,
> >> int64_t size)
> >>>   value_type = avio_rl16(pb); /* value_type */
> >>>   value_len  = avio_rl32(pb);
> >>>
> >>> -if (value_len < 0 || value_len > UINT16_MAX)
> >>> +if (value_len < 0)
> >>>   return AVERROR_INVALIDDATA;
> >>
> >> I may misread the code but it appears to me that an assert can be
> >> triggered now, no?
> >
> > Right, for these 11 discrete size values only, though:
> >
> > 2147483647, 2147483646, 2147483645, 2147483644, 2147483643,
> > 2147483642, 2147483641, 2147483640, 2147483639, 2147483638
> > 2147483636
> >
> > A chance of 11 in 4 Billions :-)
> 
> len < (INT_MAX - LEN) / 2 is more than half the range of an int.

I've been one step ahead in calculation:

av_malloc takes size_t and on 32bit platforms, this is uint32, which means
that it can take a maximum of 4.294.967.295.
(4.294.967.295 - 22) / 2 = 2.147.483.636  (+ 11 makes INT32_MAX)

> 
> >
> > TBH, I don't understand this assert. When the attachment is too large
> > to handle, we should rather log a warning and goto finish instead IMO.
> 
> The assert is to ensure get_tag() is not called with out of range values, so 
> the
> relevant checks should happen before the call.

I'm not sure whether I'd agree with that. It shouldn't be the caller 
needing to know what the function can handle and what it can't handle.

The INT32/2 is not only wrong, it also does _only_ apply to reading 
unicode. For ASCII and byte arrays, there is no need for this restriction.
And on 64bit platforms, there's no need for this that restriction at all.

softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] fftools/cmdutils: don't print build configuration by default

2021-08-07 Thread Carl Eugen Hoyos
Am Sa., 7. Aug. 2021 um 18:14 Uhr schrieb James Almer :
>
> On 8/7/2021 1:06 PM, Derek Buitenhuis wrote:
> > On 8/6/2021 7:04 PM, James Almer wrote:
> >> From: Matthieu Patou 
> >>
> >> Suggested-by: ffm...@fb.com
> >> Signed-off-by: James Almer 
> >> ---
> >>   fftools/cmdutils.c | 5 +++--
> >>   1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > What exactly is the point of this?
> >
> > You've provided a patch with no explanation for its value.
> >
> > If it's "security", then that's kind of dumb, since if you
> > have access to run ffmpeg on something, you have access to
> > `strings $(which ffmpeg)` too.
>
> It's just to reduce the amount of information printed by default. Most
> command line utilities don't bother showing build time configuration
> options outside of --version or --help output, if at all.
> And the string is obviously in the binary (duplicated on every library
> at that), so it's not about hiding anything.
>
> I made the latest version print it on verbose level or higher (and
> always with --version), which may be preferable. But i nonetheless
> don't have a strong opinion about this change.

FWIW, the change is a very bad idea imo.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread James Almer

On 8/7/2021 9:38 PM, Soft Works wrote:

-Original Message-
From: ffmpeg-devel  On Behalf Of
Carl Eugen Hoyos
Sent: Sunday, 8 August 2021 01:58
To: FFmpeg development discussions and patches 
Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression
bug when reading image attachments

Am So., 8. Aug. 2021 um 01:53 Uhr schrieb Soft Works
:


Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a

check for value_len > UINT16_MAX.

As a consequence, attached images of sizes larger than UINT16_MAX could

no longer be read.


Signed-off-by: softworkz 
---
v2: Fix without changing variable type  libavformat/asfdec_f.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c index
ff6ddfb967..b9f3918495 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -614,7 +614,7 @@ static int asf_read_metadata(AVFormatContext *s,

int64_t size)

  value_type = avio_rl16(pb); /* value_type */
  value_len  = avio_rl32(pb);

-if (value_len < 0 || value_len > UINT16_MAX)
+if (value_len < 0)
  return AVERROR_INVALIDDATA;


I may misread the code but it appears to me that an assert can be triggered
now, no?


Right, for these 11 discrete size values only, though:

2147483647, 2147483646, 2147483645, 2147483644, 2147483643,
2147483642, 2147483641, 2147483640, 2147483639, 2147483638
2147483636

A chance of 11 in 4 Billions :-)


len < (INT_MAX - LEN) / 2 is more than half the range of an int.



TBH, I don't understand this assert. When the attachment is too large
to handle, we should rather log a warning and goto finish instead IMO.


The assert is to ensure get_tag() is not called with out of range 
values, so the relevant checks should happen before the call.




softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread Soft Works
> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Carl Eugen Hoyos
> Sent: Sunday, 8 August 2021 01:58
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression
> bug when reading image attachments
> 
> Am So., 8. Aug. 2021 um 01:53 Uhr schrieb Soft Works
> :
> >
> > Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a
> check for value_len > UINT16_MAX.
> > As a consequence, attached images of sizes larger than UINT16_MAX could
> no longer be read.
> >
> > Signed-off-by: softworkz 
> > ---
> > v2: Fix without changing variable type  libavformat/asfdec_f.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c index
> > ff6ddfb967..b9f3918495 100644
> > --- a/libavformat/asfdec_f.c
> > +++ b/libavformat/asfdec_f.c
> > @@ -614,7 +614,7 @@ static int asf_read_metadata(AVFormatContext *s,
> int64_t size)
> >  value_type = avio_rl16(pb); /* value_type */
> >  value_len  = avio_rl32(pb);
> >
> > -if (value_len < 0 || value_len > UINT16_MAX)
> > +if (value_len < 0)
> >  return AVERROR_INVALIDDATA;
> 
> I may misread the code but it appears to me that an assert can be triggered
> now, no?

Right, for these 11 discrete size values only, though:

2147483647, 2147483646, 2147483645, 2147483644, 2147483643,
2147483642, 2147483641, 2147483640, 2147483639, 2147483638
2147483636

A chance of 11 in 4 Billions :-)

TBH, I don't understand this assert. When the attachment is too large
to handle, we should rather log a warning and goto finish instead IMO.

softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread Carl Eugen Hoyos
Am So., 8. Aug. 2021 um 01:33 Uhr schrieb Carl Eugen Hoyos :
>
> Am So., 8. Aug. 2021 um 01:26 Uhr schrieb Soft Works :
> >
> > Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a check for 
> > value_len > UINT16_MAX.
> > As a consequence, attached images of sizes larger than UINT16_MAX could no 
> > longer be read.
> >
> > Signed-off-by: softworkz 
> > ---
> >  libavformat/asfdec_f.c | 7 ---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
> > index f784e62996..708331637e 100644
> > --- a/libavformat/asfdec_f.c
> > +++ b/libavformat/asfdec_f.c
> > @@ -707,7 +707,8 @@ static int asf_read_metadata(AVFormatContext *s, 
> > int64_t size)
> >  {
> >  AVIOContext *pb = s->pb;
> >  ASFContext *asf = s->priv_data;
> > -int n, stream_num, name_len_utf16, name_len_utf8, value_len;
> > +int n, stream_num, name_len_utf16, name_len_utf8;
> > +   unsigned int value_len;
>
> There is something wrong with the indentation afaict.

> And why can't you fix the issue leaving the variable an int?

Should have been:
Is it possible to fix the issue without changing the variable type?

Sorry for the bad wording, Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread Carl Eugen Hoyos
Am So., 8. Aug. 2021 um 01:53 Uhr schrieb Soft Works :
>
> Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a check for 
> value_len > UINT16_MAX.
> As a consequence, attached images of sizes larger than UINT16_MAX could no 
> longer be read.
>
> Signed-off-by: softworkz 
> ---
> v2: Fix without changing variable type
>  libavformat/asfdec_f.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
> index ff6ddfb967..b9f3918495 100644
> --- a/libavformat/asfdec_f.c
> +++ b/libavformat/asfdec_f.c
> @@ -614,7 +614,7 @@ static int asf_read_metadata(AVFormatContext *s, int64_t 
> size)
>  value_type = avio_rl16(pb); /* value_type */
>  value_len  = avio_rl32(pb);
>
> -if (value_len < 0 || value_len > UINT16_MAX)
> +if (value_len < 0)
>  return AVERROR_INVALIDDATA;

I may misread the code but it appears to me that an assert
can be triggered now, no?

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread Soft Works



> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Soft Works
> Sent: Sunday, 8 August 2021 01:53
> To: ffmpeg-devel@ffmpeg.org
> Subject: [FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression bug
> when reading image attachments
> 
> Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a check
> for value_len > UINT16_MAX.
> As a consequence, attached images of sizes larger than UINT16_MAX could
> no longer be read.
> 
> Signed-off-by: softworkz 
> ---
> v2: Fix without changing variable type
>  libavformat/asfdec_f.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c index
> ff6ddfb967..b9f3918495 100644
> --- a/libavformat/asfdec_f.c
> +++ b/libavformat/asfdec_f.c
> @@ -614,7 +614,7 @@ static int asf_read_metadata(AVFormatContext *s,
> int64_t size)
>  value_type = avio_rl16(pb); /* value_type */
>  value_len  = avio_rl32(pb);
> 
> -if (value_len < 0 || value_len > UINT16_MAX)
> +if (value_len < 0)
>  return AVERROR_INVALIDDATA;
> 
>  name_len_utf8 = 2*name_len_utf16 + 1;
> --
> 2.28.0.windows.1

@Carl - I'm flexible, though. Better now?

softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v2] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread Soft Works
Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a check for 
value_len > UINT16_MAX.
As a consequence, attached images of sizes larger than UINT16_MAX could no 
longer be read.

Signed-off-by: softworkz 
---
v2: Fix without changing variable type
 libavformat/asfdec_f.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index ff6ddfb967..b9f3918495 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -614,7 +614,7 @@ static int asf_read_metadata(AVFormatContext *s, int64_t 
size)
 value_type = avio_rl16(pb); /* value_type */
 value_len  = avio_rl32(pb);
 
-if (value_len < 0 || value_len > UINT16_MAX)
+if (value_len < 0)
 return AVERROR_INVALIDDATA;
 
 name_len_utf8 = 2*name_len_utf16 + 1;
-- 
2.28.0.windows.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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread Soft Works
> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Carl Eugen Hoyos
> Sent: Sunday, 8 August 2021 01:33
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] libavformat/asfdec: Fix regression bug
> when reading image attachments
> 
> Am So., 8. Aug. 2021 um 01:26 Uhr schrieb Soft Works
> :
> >
> > Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a
> check for value_len > UINT16_MAX.
> > As a consequence, attached images of sizes larger than UINT16_MAX could
> no longer be read.
> >
> > Signed-off-by: softworkz 
> > ---
> >  libavformat/asfdec_f.c | 7 ---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c index
> > f784e62996..708331637e 100644
> > --- a/libavformat/asfdec_f.c
> > +++ b/libavformat/asfdec_f.c
> > @@ -707,7 +707,8 @@ static int asf_read_metadata(AVFormatContext *s,
> > int64_t size)  {
> >  AVIOContext *pb = s->pb;
> >  ASFContext *asf = s->priv_data;
> > -int n, stream_num, name_len_utf16, name_len_utf8, value_len;
> > +int n, stream_num, name_len_utf16, name_len_utf8;
> > +   unsigned int value_len;
> 
> There is something wrong with the indentation afaict.

You're right, wrong IDE setting. Thanks.

> And why can't you fix the issue leaving the variable an int?

Why use an int variable to take the value of a function that 
returns uint and afterwards check whether it's negative?
Why use int type for size values at all?

I didn't want to make a bigger thing out of the fix, though.

softworkz
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] mxf : correct previous commit

2021-08-07 Thread Carl Eugen Hoyos
Am Do., 5. Aug. 2021 um 11:03 Uhr schrieb Michael Krebs
:
>
> * Let older tags on the same place as originally
> * Add new fate tests for rawvideo and v210 and update checksum for mxf tests

This commit message indicates that the patch should be split.

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] Nvenc: Adding support for chroma qp offset for h264_nvenc and hevc_nvenc

2021-08-07 Thread Timo Rothenpieler

On 03.08.2021 17:14, Ricardo Monteiro wrote:

Hi Timo,


I do wonder if that information isn't (also) available via metadata on
the incoming frames, so it could be inferred from those by default.


Are you talking about a transcoding scenario? Where you would load the 
parameters from the input bitstream? I'm not sure how useful this would be 
though.


Other than that, this will need the usual header-version-guards so it
doesn't break compatiblity with older headers and hence older drivers.


I updated the patch (attached).


I wonder, wouldn't it make more sense to move setting these from the two 
codec specific functions to set_constqp()?

Or are they useful in any other RC mode?

You don't need to send a new patch for that, I can just move them there 
before applying.



Timo



smime.p7s
Description: S/MIME Cryptographic Signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread Carl Eugen Hoyos
Am So., 8. Aug. 2021 um 01:26 Uhr schrieb Soft Works :
>
> Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a check for 
> value_len > UINT16_MAX.
> As a consequence, attached images of sizes larger than UINT16_MAX could no 
> longer be read.
>
> Signed-off-by: softworkz 
> ---
>  libavformat/asfdec_f.c | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
> index f784e62996..708331637e 100644
> --- a/libavformat/asfdec_f.c
> +++ b/libavformat/asfdec_f.c
> @@ -707,7 +707,8 @@ static int asf_read_metadata(AVFormatContext *s, int64_t 
> size)
>  {
>  AVIOContext *pb = s->pb;
>  ASFContext *asf = s->priv_data;
> -int n, stream_num, name_len_utf16, name_len_utf8, value_len;
> +int n, stream_num, name_len_utf16, name_len_utf8;
> +   unsigned int value_len;

There is something wrong with the indentation afaict.

And why can't you fix the issue leaving the variable an int?

Carl Eugen
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH] libavformat/asfdec: Fix regression bug when reading image attachments

2021-08-07 Thread Soft Works
Commit c8140fe7324f264faacf7395b27e12531d1f13f7 had introduced a check for 
value_len > UINT16_MAX.
As a consequence, attached images of sizes larger than UINT16_MAX could no 
longer be read.

Signed-off-by: softworkz 
---
 libavformat/asfdec_f.c | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libavformat/asfdec_f.c b/libavformat/asfdec_f.c
index f784e62996..708331637e 100644
--- a/libavformat/asfdec_f.c
+++ b/libavformat/asfdec_f.c
@@ -707,7 +707,8 @@ static int asf_read_metadata(AVFormatContext *s, int64_t 
size)
 {
 AVIOContext *pb = s->pb;
 ASFContext *asf = s->priv_data;
-int n, stream_num, name_len_utf16, name_len_utf8, value_len;
+int n, stream_num, name_len_utf16, name_len_utf8;
+   unsigned int value_len;
 int ret, i;
 n = avio_rl16(pb);
 
@@ -721,7 +722,7 @@ static int asf_read_metadata(AVFormatContext *s, int64_t 
size)
 value_type = avio_rl16(pb); /* value_type */
 value_len  = avio_rl32(pb);
 
-if (value_len < 0 || value_len > UINT16_MAX)
+if (value_len > INT32_MAX)
 return AVERROR_INVALIDDATA;
 
 name_len_utf8 = 2*name_len_utf16 + 1;
@@ -743,7 +744,7 @@ static int asf_read_metadata(AVFormatContext *s, int64_t 
size)
 if(stream_num < 128)
 asf->dar[stream_num].den = aspect_y;
 } else {
-get_tag(s, name, value_type, value_len, 16);
+get_tag(s, name, value_type, (int)value_len, 16);
 }
 av_freep();
 }
-- 
2.28.0.windows.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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] avcodec/ass_split: Rename ff_ass_split_dialog2->ff_ass_split_dialog

2021-08-07 Thread James Almer

On 8/7/2021 6:54 PM, Andreas Rheinhardt wrote:

Signed-off-by: Andreas Rheinhardt 
---
This is in reply to James' opinion here:
https://ffmpeg.org/pipermail/ffmpeg-devel/2021-August/283267.html

  libavcodec/ass_split.c  | 2 +-
  libavcodec/ass_split.h  | 4 ++--
  libavcodec/movtextenc.c | 2 +-
  libavcodec/srtenc.c | 2 +-
  libavcodec/ttmlenc.c| 2 +-
  libavcodec/webvttenc.c  | 2 +-
  6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/libavcodec/ass_split.c b/libavcodec/ass_split.c
index eb7ff2845a..05c5453e53 100644
--- a/libavcodec/ass_split.c
+++ b/libavcodec/ass_split.c
@@ -424,7 +424,7 @@ void ff_ass_free_dialog(ASSDialog **dialogp)
  av_freep(dialogp);
  }
  
-ASSDialog *ff_ass_split_dialog2(ASSSplitContext *ctx, const char *buf)

+ASSDialog *ff_ass_split_dialog(ASSSplitContext *ctx, const char *buf)
  {
  int i;
  static const ASSFields fields[] = {
diff --git a/libavcodec/ass_split.h b/libavcodec/ass_split.h
index 2ce756203e..a45fb9b8a1 100644
--- a/libavcodec/ass_split.h
+++ b/libavcodec/ass_split.h
@@ -110,7 +110,7 @@ typedef struct ASSSplitContext ASSSplitContext;
  ASSSplitContext *ff_ass_split(const char *buf);
  
  /**

- * Free a dialogue obtained from ff_ass_split_dialog2().
+ * Free a dialogue obtained from ff_ass_split_dialog().
   */
  void ff_ass_free_dialog(ASSDialog **dialogp);
  
@@ -121,7 +121,7 @@ void ff_ass_free_dialog(ASSDialog **dialogp);

   * @param buf String containing the ASS "Dialogue" line.
   * @return Pointer to the split ASSDialog. Must be freed with 
ff_ass_free_dialog()
   */
-ASSDialog *ff_ass_split_dialog2(ASSSplitContext *ctx, const char *buf);
+ASSDialog *ff_ass_split_dialog(ASSSplitContext *ctx, const char *buf);
  
  /**

   * Free all the memory allocated for an ASSSplitContext.
diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c
index a6a1808592..2ae5a9bf0b 100644
--- a/libavcodec/movtextenc.c
+++ b/libavcodec/movtextenc.c
@@ -654,7 +654,7 @@ static int mov_text_encode_frame(AVCodecContext *avctx, 
unsigned char *buf,
  return AVERROR(EINVAL);
  }
  
-dialog = ff_ass_split_dialog2(s->ass_ctx, ass);

+dialog = ff_ass_split_dialog(s->ass_ctx, ass);
  if (!dialog)
  return AVERROR(ENOMEM);
  mov_text_dialog(s, dialog);
diff --git a/libavcodec/srtenc.c b/libavcodec/srtenc.c
index edc91c4013..2e3ac55770 100644
--- a/libavcodec/srtenc.c
+++ b/libavcodec/srtenc.c
@@ -245,7 +245,7 @@ static int encode_frame(AVCodecContext *avctx,
  return AVERROR(EINVAL);
  }
  
-dialog = ff_ass_split_dialog2(s->ass_ctx, ass);

+dialog = ff_ass_split_dialog(s->ass_ctx, ass);
  if (!dialog)
  return AVERROR(ENOMEM);
  s->alignment_applied = 0;
diff --git a/libavcodec/ttmlenc.c b/libavcodec/ttmlenc.c
index 5cab33cc60..ad2eddfdd5 100644
--- a/libavcodec/ttmlenc.c
+++ b/libavcodec/ttmlenc.c
@@ -95,7 +95,7 @@ static int ttml_encode_frame(AVCodecContext *avctx, uint8_t 
*buf,
  return AVERROR(EINVAL);
  }
  
-dialog = ff_ass_split_dialog2(s->ass_ctx, ass);

+dialog = ff_ass_split_dialog(s->ass_ctx, ass);
  if (!dialog)
  return AVERROR(ENOMEM);
  
diff --git a/libavcodec/webvttenc.c b/libavcodec/webvttenc.c

index 3181e25120..89b49e42bf 100644
--- a/libavcodec/webvttenc.c
+++ b/libavcodec/webvttenc.c
@@ -172,7 +172,7 @@ static int webvtt_encode_frame(AVCodecContext *avctx,
  return AVERROR(EINVAL);
  }
  
-dialog = ff_ass_split_dialog2(s->ass_ctx, ass);

+dialog = ff_ass_split_dialog(s->ass_ctx, ass);
  if (!dialog)
  return AVERROR(ENOMEM);
  webvtt_style_apply(s, dialog->style);


Should be ok.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 1/3] avfilter/vf_vif: Fix mismatch in number of array elements

2021-08-07 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> The function definition used float *data_buf[14], although there are
> only 13 elements (and only 13 are used); the declaration used 13.
> Given that the type will be converted to float **data_buf anyway,
> this is not in violation of the C specs, but nevertheless a bug.
> 
> GCC 11 has a new warning for this -Warray-parameter.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavfilter/vf_vif.c | 10 ++
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/libavfilter/vf_vif.c b/libavfilter/vf_vif.c
> index a136d038fb..bb949649e4 100644
> --- a/libavfilter/vf_vif.c
> +++ b/libavfilter/vf_vif.c
> @@ -38,6 +38,8 @@
>  #include "vif.h"
>  #include "video.h"
>  
> +#define NUM_DATA_BUFS 13
> +
>  typedef struct VIFContext {
>  const AVClass *class;
>  FFFrameSync fs;
> @@ -46,7 +48,7 @@ typedef struct VIFContext {
>  int height;
>  int nb_threads;
>  float factor;
> -float *data_buf[13];
> +float *data_buf[NUM_DATA_BUFS];
>  float **temp;
>  float *ref_data;
>  float *main_data;
> @@ -286,7 +288,7 @@ static int vif_filter1d(AVFilterContext *ctx, void *arg, 
> int jobnr, int nb_jobs)
>  int ff_compute_vif2(AVFilterContext *ctx,
>  const float *ref, const float *main, int w, int h,
>  int ref_stride, int main_stride, float *score,
> -float *data_buf[14], float **temp,
> +float *data_buf[NUM_DATA_BUFS], float **temp,
>  int gnb_threads)
>  {
>  ThreadData td;
> @@ -515,7 +517,7 @@ static int config_input_ref(AVFilterLink *inlink)
>  s->vif_max[i] = -DBL_MAX;
>  }
>  
> -for (int i = 0; i < 13; i++) {
> +for (int i = 0; i < NUM_DATA_BUFS; i++) {
>  if (!(s->data_buf[i] = av_calloc(s->width, s->height * 
> sizeof(float
>  return AVERROR(ENOMEM);
>  }
> @@ -608,7 +610,7 @@ static av_cold void uninit(AVFilterContext *ctx)
> i, s->vif_sum[i] / s->nb_frames, s->vif_min[i], 
> s->vif_max[i]);
>  }
>  
> -for (int i = 0; i < 13; i++)
> +for (int i = 0; i < NUM_DATA_BUFS; i++)
>  av_freep(>data_buf[i]);
>  
>  av_freep(>ref_data);
> 
Will apply the rest of this patchset tomorrow unless there are objections.

- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH] avcodec/ass_split: Rename ff_ass_split_dialog2->ff_ass_split_dialog

2021-08-07 Thread Andreas Rheinhardt
Signed-off-by: Andreas Rheinhardt 
---
This is in reply to James' opinion here:
https://ffmpeg.org/pipermail/ffmpeg-devel/2021-August/283267.html

 libavcodec/ass_split.c  | 2 +-
 libavcodec/ass_split.h  | 4 ++--
 libavcodec/movtextenc.c | 2 +-
 libavcodec/srtenc.c | 2 +-
 libavcodec/ttmlenc.c| 2 +-
 libavcodec/webvttenc.c  | 2 +-
 6 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/libavcodec/ass_split.c b/libavcodec/ass_split.c
index eb7ff2845a..05c5453e53 100644
--- a/libavcodec/ass_split.c
+++ b/libavcodec/ass_split.c
@@ -424,7 +424,7 @@ void ff_ass_free_dialog(ASSDialog **dialogp)
 av_freep(dialogp);
 }
 
-ASSDialog *ff_ass_split_dialog2(ASSSplitContext *ctx, const char *buf)
+ASSDialog *ff_ass_split_dialog(ASSSplitContext *ctx, const char *buf)
 {
 int i;
 static const ASSFields fields[] = {
diff --git a/libavcodec/ass_split.h b/libavcodec/ass_split.h
index 2ce756203e..a45fb9b8a1 100644
--- a/libavcodec/ass_split.h
+++ b/libavcodec/ass_split.h
@@ -110,7 +110,7 @@ typedef struct ASSSplitContext ASSSplitContext;
 ASSSplitContext *ff_ass_split(const char *buf);
 
 /**
- * Free a dialogue obtained from ff_ass_split_dialog2().
+ * Free a dialogue obtained from ff_ass_split_dialog().
  */
 void ff_ass_free_dialog(ASSDialog **dialogp);
 
@@ -121,7 +121,7 @@ void ff_ass_free_dialog(ASSDialog **dialogp);
  * @param buf String containing the ASS "Dialogue" line.
  * @return Pointer to the split ASSDialog. Must be freed with 
ff_ass_free_dialog()
  */
-ASSDialog *ff_ass_split_dialog2(ASSSplitContext *ctx, const char *buf);
+ASSDialog *ff_ass_split_dialog(ASSSplitContext *ctx, const char *buf);
 
 /**
  * Free all the memory allocated for an ASSSplitContext.
diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c
index a6a1808592..2ae5a9bf0b 100644
--- a/libavcodec/movtextenc.c
+++ b/libavcodec/movtextenc.c
@@ -654,7 +654,7 @@ static int mov_text_encode_frame(AVCodecContext *avctx, 
unsigned char *buf,
 return AVERROR(EINVAL);
 }
 
-dialog = ff_ass_split_dialog2(s->ass_ctx, ass);
+dialog = ff_ass_split_dialog(s->ass_ctx, ass);
 if (!dialog)
 return AVERROR(ENOMEM);
 mov_text_dialog(s, dialog);
diff --git a/libavcodec/srtenc.c b/libavcodec/srtenc.c
index edc91c4013..2e3ac55770 100644
--- a/libavcodec/srtenc.c
+++ b/libavcodec/srtenc.c
@@ -245,7 +245,7 @@ static int encode_frame(AVCodecContext *avctx,
 return AVERROR(EINVAL);
 }
 
-dialog = ff_ass_split_dialog2(s->ass_ctx, ass);
+dialog = ff_ass_split_dialog(s->ass_ctx, ass);
 if (!dialog)
 return AVERROR(ENOMEM);
 s->alignment_applied = 0;
diff --git a/libavcodec/ttmlenc.c b/libavcodec/ttmlenc.c
index 5cab33cc60..ad2eddfdd5 100644
--- a/libavcodec/ttmlenc.c
+++ b/libavcodec/ttmlenc.c
@@ -95,7 +95,7 @@ static int ttml_encode_frame(AVCodecContext *avctx, uint8_t 
*buf,
 return AVERROR(EINVAL);
 }
 
-dialog = ff_ass_split_dialog2(s->ass_ctx, ass);
+dialog = ff_ass_split_dialog(s->ass_ctx, ass);
 if (!dialog)
 return AVERROR(ENOMEM);
 
diff --git a/libavcodec/webvttenc.c b/libavcodec/webvttenc.c
index 3181e25120..89b49e42bf 100644
--- a/libavcodec/webvttenc.c
+++ b/libavcodec/webvttenc.c
@@ -172,7 +172,7 @@ static int webvtt_encode_frame(AVCodecContext *avctx,
 return AVERROR(EINVAL);
 }
 
-dialog = ff_ass_split_dialog2(s->ass_ctx, ass);
+dialog = ff_ass_split_dialog(s->ass_ctx, ass);
 if (!dialog)
 return AVERROR(ENOMEM);
 webvtt_style_apply(s, dialog->style);
-- 
2.30.2

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 2/4] avcodec/ass_split: Remove unused ff_ass_split_dialogue()

2021-08-07 Thread James Almer

On 8/6/2021 10:47 PM, Andreas Rheinhardt wrote:

Unused since 1f63665ca567fbc49fa80166d468a822c2999efa.

Signed-off-by: Andreas Rheinhardt 
---
Shall I rename ff_ass_split_dialogue2 to ff_ass_split_dialogue?


IMO, yes.



  libavcodec/ass_split.c | 19 ---
  libavcodec/ass_split.h | 16 
  2 files changed, 35 deletions(-)

diff --git a/libavcodec/ass_split.c b/libavcodec/ass_split.c
index ea3008ce91..5e3883fea4 100644
--- a/libavcodec/ass_split.c
+++ b/libavcodec/ass_split.c
@@ -418,25 +418,6 @@ static void free_section(ASSSplitContext *ctx, const 
ASSSection *section)
  av_freep((uint8_t *)>ass + section->offset);
  }
  
-ASSDialog *ff_ass_split_dialog(ASSSplitContext *ctx, const char *buf,

-   int cache, int *number)
-{
-ASSDialog *dialog = NULL;
-int i, count;
-if (!cache)
-for (i=0; iass.dialogs_count;
-if (ass_split(ctx, buf) == 0)
-dialog = ctx->ass.dialogs + count;
-if (number)
-*number = ctx->ass.dialogs_count - count;
-return dialog;
-}
-
  void ff_ass_free_dialog(ASSDialog **dialogp)
  {
  ASSDialog *dialog = *dialogp;
diff --git a/libavcodec/ass_split.h b/libavcodec/ass_split.h
index 30ce77250c..2ce756203e 100644
--- a/libavcodec/ass_split.h
+++ b/libavcodec/ass_split.h
@@ -109,22 +109,6 @@ typedef struct ASSSplitContext ASSSplitContext;
   */
  ASSSplitContext *ff_ass_split(const char *buf);
  
-/**

- * Split one or several ASS "Dialogue" lines from a string buffer and store
- * them in an already initialized context.
- *
- * @param ctx Context previously initialized by ff_ass_split().
- * @param buf String containing the ASS "Dialogue" lines.
- * @param cache Set to 1 to keep all the previously split ASSDialog in
- *  the context, or set to 0 to free all the previously split
- *  ASSDialog.
- * @param number If not NULL, the pointed integer will be set to the number
- *   of split ASSDialog.
- * @return Pointer to the first split ASSDialog.
- */
-ASSDialog *ff_ass_split_dialog(ASSSplitContext *ctx, const char *buf,
-   int cache, int *number);
-
  /**
   * Free a dialogue obtained from ff_ass_split_dialog2().
   */


LGTM.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 1/4] avfilter/asrc_flite: Don't define an object by accident

2021-08-07 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> The flite filter apparently only wanted to declare a struct,
> but mistakenly also defined an unused and zero-initialized element
> with external linkage.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> Would not have happened if one had actually defined a typedef.
> 
>  libavfilter/asrc_flite.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavfilter/asrc_flite.c b/libavfilter/asrc_flite.c
> index e3065cff79..1478cc07fa 100644
> --- a/libavfilter/asrc_flite.c
> +++ b/libavfilter/asrc_flite.c
> @@ -81,7 +81,7 @@ struct voice_entry {
>  void (*unregister_fn)(cst_voice *);
>  cst_voice *voice;
>  unsigned usage_count;
> -} voice_entry;
> +};
>  
>  #define MAKE_VOICE_STRUCTURE(voice_name) { \
>  .name  =  #voice_name, \
> 
Will apply this patchset tomorrow unless there are objections.

- Andreas
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2] doc/developer: Add description about safely sending patches via E-Mail clients

2021-08-07 Thread Soft Works
> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Andreas Rheinhardt
> Sent: Saturday, 7 August 2021 13:51
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v2] doc/developer: Add description
> about safely sending patches via E-Mail clients
> 
> Soft Works:
> > (v2: fix doc build)
> 
> This should be directly after the --- below.

Thanks for the hint. This mailing list thing is not my world ;-)

> >
> > Signed-off-by: softworkz 
> > ---
> >  doc/developer.texi | 16 
> >  1 file changed, 16 insertions(+)
> >
> > diff --git a/doc/developer.texi b/doc/developer.texi index
> > b33cab0fc7..ce5500b85d 100644
> > --- a/doc/developer.texi
> > +++ b/doc/developer.texi
> > @@ -494,6 +494,22 @@ patch is inline or attached per mail.
> >  You can check @url{https://patchwork.ffmpeg.org}, if your patch does
> > not show up, its mime type  likely was wrong.
> >
> > +@subheading Sending E-Mail Client - Example for MS Outlook.
> > +Using @code{git send-email} might not be desirable for everyone. The
> > +following trick allows to send patches via E-Mail clients in a safe
> > +way. It has been tested with Outlook and Thunderbird (with X-Unsent
> > +extension) and might work with other applications.
> > +
> > +Create your patch like this:
> > +
> > +@verbatim
> > +git format-patch -s -o "outputfolder" --add-header "X-Unsent: 1"
> > +--suffix .eml --to ffmpeg-devel@ffmpeg.org -1 1a2b3c4d @end verbatim
> > +
> > +Now you'll just need to open the eml file with the E-Mail application
> > +and execure 'Send'.
> 
> I guess this is supposed to be execute.

Good eye!

Thanks for reviewing. Update sent.

sw
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH v3] doc/developer: Add description about safely sending patches via E-Mail clients

2021-08-07 Thread Soft Works
Signed-off-by: softworkz 
---
v3: fixed typo
 doc/developer.texi | 16 
 1 file changed, 16 insertions(+)

diff --git a/doc/developer.texi b/doc/developer.texi
index b33cab0fc7..86f63c49b3 100644
--- a/doc/developer.texi
+++ b/doc/developer.texi
@@ -494,6 +494,22 @@ patch is inline or attached per mail.
 You can check @url{https://patchwork.ffmpeg.org}, if your patch does not show 
up, its mime type
 likely was wrong.
 
+@subheading Sending E-Mail Client - Example for MS Outlook.
+Using @code{git send-email} might not be desirable for everyone. The 
+following trick allows to send patches via E-Mail clients in a safe 
+way. It has been tested with Outlook and Thunderbird (with X-Unsent
+extension) and might work with other applications.
+
+Create your patch like this:
+
+@verbatim
+git format-patch -s -o "outputfolder" --add-header "X-Unsent: 1" --suffix .eml 
--to ffmpeg-devel@ffmpeg.org -1 1a2b3c4d
+@end verbatim
+
+Now you'll just need to open the eml file with the E-Mail application
+and execute 'Send'.
+
+@subheading Reviews.
 Your patch will be reviewed on the mailing list. You will likely be asked
 to make some changes and are expected to send in an improved version that
 incorporates the requests from the review. This process may go through
-- 
2.28.0.windows.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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 1/3] Revert "fftools/ffmpeg_filter: fix the flags parsing for scaler"

2021-08-07 Thread Michael Niedermayer
On Sat, Aug 07, 2021 at 06:15:05PM +0800, Linjie Fu wrote:
> From: Linjie Fu 
> 
> This reverts commit b3a0548a981db52911dd34d9de254c4fee0a8f79.

LGTM

thx

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

Those who would give up essential Liberty, to purchase a little
temporary Safety, deserve neither Liberty nor Safety -- Benjamin Franklin


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 4/4] avcodec/acelp_vectors: Add missing brackets

2021-08-07 Thread Michael Niedermayer
On Fri, Aug 06, 2021 at 06:17:25PM +0200, Andreas Rheinhardt wrote:
> Before 3793caa5e2d1d16ed45771574b2ffc932497cfcf the code was
> "if (...) do { ... } while (...);". After said commit this became
> "if (...) av_assert0(...); do { ... } while (...);", i.e. the loop
> is always executed. This commit changes the logic to what it was before
> said commit. Notice that the condition is always true in FATE, so no
> changes are necessary there.
> 
> This fixes a -Wmisleading-indentation warning from GCC 11.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/acelp_vectors.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

LGTM

thx

[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2] avcodec/mpeg12dec: report error when picture type is unknown and err_detect is EXPLODE

2021-08-07 Thread Marton Balint




On Sat, 7 Aug 2021, Michael Niedermayer wrote:


On Sat, Aug 07, 2021 at 09:21:36AM +0200, Marton Balint wrote:



On Fri, 6 Aug 2021, Michael Niedermayer wrote:


On Mon, Aug 02, 2021 at 08:50:05PM +0200, Marton Balint wrote:

Also split error message to error and warning.

Signed-off-by: Marton Balint 
---
 libavcodec/mpeg12dec.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 858dca660c..49d865853b 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -1529,7 +1529,7 @@ static void 
mpeg_decode_quant_matrix_extension(MpegEncContext *s)
 load_matrix(s, s->chroma_inter_matrix, NULL, 0);
 }

-static void mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
+static int mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
 {
 MpegEncContext *s = >mpeg_enc_ctx;

@@ -1539,8 +1539,10 @@ static void 
mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
 s->mpeg_f_code[1][0] = get_bits(>gb, 4);
 s->mpeg_f_code[1][1] = get_bits(>gb, 4);
 if (!s->pict_type && s1->mpeg_enc_ctx_allocated) {
-av_log(s->avctx, AV_LOG_ERROR,
-   "Missing picture start code, guessing missing values\n");
+av_log(s->avctx, AV_LOG_ERROR, "Missing picture start code\n");
+if (s->avctx->err_recognition & AV_EF_EXPLODE)
+return AVERROR_INVALIDDATA;
+av_log(s->avctx, AV_LOG_WARNING, "Guessing pict_type from 
mpeg_f_code\n");


If we are nitpicking then this is not ideal
"Missing picture start code" is only an error when AV_EF_EXPLODE is set
because it only in that case results in wrong output, when AV_EF_EXPLODE
is not set the output from the decoder might be correct si it too should
be a warning in that case


I understand your logic, but there are arguments for keeping the error
message as well. I think in practice, most times, this error is caused by
bitstream errors and it _will_ affect decoding.



Also there are tons of cases
in the code which log an error, then return INVALIDDATA if EXPLODE is used,
so why handle it here differently...


do these cases really represent situations where the output might be correct ?
for examlple any out of range coefficient cant be correct and a mismatching CRC
well it "could" be.
I think a big part would be, "do we actually have cases where it is not an
error ?" if so i would argue that it should be a warning


Obviously it is hard to be sure. Among the samples we have, I found a few 
files affected:


ffmpeg-bugs/trac/ticket2705/81391.mpg
ffmpeg-bugs/trac/ticket2950/mpeg2_fuzz.mpg
ffmpeg-bugs/roundup/issue1872/lol-mpeg2.mp4
ffmpeg-bugs/roundup/issue1218/dvbt-ffmpeg-seg-fault.ts

All are heavily corrupted files.

I also had a private sample file having this issue, and no other error 
but the "ignoring extra picture following a frame-picture" warning, and 
it is also a corrupted bitstream, and causes missed frames when decoding.


So a patologcal bitstream can probably be created when the decoding is not 
affected, but my guess is that for real-world files decoding is always 
affected, therefore I think an error is more appropriate.


Let me know what you prefer.

Thanks,
Marton





But let me know if you insisit and I will change it. And yes, it is indeed
nitpicking :)


i dont insist but thats how the levels are defined

/**
* Something went wrong and cannot losslessly be recovered.
* However, not all future data is affected.
*/
#define AV_LOG_ERROR16

/**
* Something somehow does not look correct. This may or may not
* lead to problems. An example would be the use of '-vstrict -2'.
*/
#define AV_LOG_WARNING  24


[...]

--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"I am not trying to be anyone's saviour, I'm trying to think about the
future and not be sad" - Elon Musk



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2 4/4] avdevice/decklink: support for more duplex mode for Decklink 8K Pro

2021-08-07 Thread Marton Balint




On Fri, 6 Aug 2021, lance.lmw...@gmail.com wrote:


From: Limin Wang 

Signed-off-by: Limin Wang 
---
doc/indevs.texi | 16 +++-
doc/outdevs.texi| 16 +++-
libavdevice/decklink_common.cpp |  8 
libavdevice/decklink_common.h   | 11 +++
libavdevice/decklink_dec_c.c| 10 ++
libavdevice/decklink_enc_c.c| 10 ++
6 files changed, 69 insertions(+), 2 deletions(-)

diff --git a/doc/indevs.texi b/doc/indevs.texi
index b377924..af0380a 100644
--- a/doc/indevs.texi
+++ b/doc/indevs.texi
@@ -344,9 +344,23 @@ Defines number of audio channels to capture. Must be 
@samp{2}, @samp{8} or @samp
Defaults to @samp{2}.

@item duplex_mode
-Sets the decklink device duplex mode. Must be @samp{unset}, @samp{half} or 
@samp{full}.
+Sets the decklink device duplex/profile mode. Must be @samp{unset}, 
@samp{half}, @samp{full},
+@samp{one_sub_device_full}, @samp{one_sub_device_half}, 
@samp{two_sub_device_full},
+@samp{four_sub_device_half}
Defaults to @samp{unset}.

+Note: DeckLink SDK 11.2 have replaced the duplex property by a profile 
property.


Why do you think it is 11.2? Accoring to the docs, it was changed in 11.0, 
and original code also had #defines for it and checked for 11.0 not 11.2. 
But your defines below check 11.2 when in fact they should check 11.0. Or 
am I missing something?


Thanks,
Marton


+For the DeckLink Duo 2 and DeckLink Quad 2, a profile is shared between any 2
+sub-devices that utilize the same connectors. For the DeckLink 8K Pro, a 
profile
+is shared between all 4 sub-devices. So DeckLink 8K Pro support four profiles.
+
+Valid profile mode for DeckLink 8K Pro(Updated DeckLink SDK to >= 11.2):
+@samp{one_sub_device_full}, @samp{one_sub_device_half}, 
@samp{two_sub_device_full},
+@samp{four_sub_device_half}
+
+Valid profile mode for DeckLink Quad 2 and DeckLink Duo 2:
+@samp{half}, @samp{full}
+
@item timecode_format
Timecode type to include in the frame and video stream metadata. Must be
@samp{none}, @samp{rp188vitc}, @samp{rp188vitc2}, @samp{rp188ltc},
diff --git a/doc/outdevs.texi b/doc/outdevs.texi
index dee9de3..76a9d7d 100644
--- a/doc/outdevs.texi
+++ b/doc/outdevs.texi
@@ -198,9 +198,23 @@ Amount of time to preroll video in seconds.
Defaults to @option{0.5}.

@item duplex_mode
-Sets the decklink device duplex mode. Must be @samp{unset}, @samp{half} or 
@samp{full}.
+Sets the decklink device duplex/profile mode. Must be @samp{unset}, 
@samp{half}, @samp{full},
+@samp{one_sub_device_full}, @samp{one_sub_device_half}, 
@samp{two_sub_device_full},
+@samp{four_sub_device_half}
Defaults to @samp{unset}.

+Note: DeckLink SDK 11.2 have replaced the duplex property by a profile 
property.
+For the DeckLink Duo 2 and DeckLink Quad 2, a profile is shared between any 2
+sub-devices that utilize the same connectors. For the DeckLink 8K Pro, a 
profile
+is shared between all 4 sub-devices. So DeckLink 8K Pro support four profiles.
+
+Valid profile mode for DeckLink 8K Pro(Updated DeckLink SDK to >= 11.2):
+@samp{one_sub_device_full}, @samp{one_sub_device_half}, 
@samp{two_sub_device_full},
+@samp{four_sub_device_half}
+
+Valid profile mode for DeckLink Quad 2 and DeckLink Duo 2:
+@samp{half}, @samp{full}
+
@item timing_offset
Sets the genlock timing pixel offset on the used output.
Defaults to @samp{unset}.
diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
index 46e9768..de7d2f4 100644
--- a/libavdevice/decklink_common.cpp
+++ b/libavdevice/decklink_common.cpp
@@ -182,7 +182,11 @@ int ff_decklink_set_configs(AVFormatContext *avctx,
if (duplex_supported) {
#if BLACKMAGIC_DECKLINK_API_VERSION >= 0x0b00
IDeckLinkProfile *profile = NULL;
+#if BLACKMAGIC_DECKLINK_API_VERSION >= 0x0b02
+BMDProfileID bmd_profile_id = 
decklink_profile_id_map[ctx->duplex_mode];
+#else
BMDProfileID bmd_profile_id = ctx->duplex_mode == 2 ? 
bmdProfileOneSubDeviceFullDuplex : bmdProfileTwoSubDevicesHalfDuplex;
+#endif
res = manager->GetProfile(bmd_profile_id, );
if (res == S_OK) {
res = profile->SetActive();
@@ -195,7 +199,11 @@ int ff_decklink_set_configs(AVFormatContext *avctx,
if (res != S_OK)
av_log(avctx, AV_LOG_WARNING, "Setting duplex mode failed.\n");
else
+#if BLACKMAGIC_DECKLINK_API_VERSION >= 0x0b02
+av_log(avctx, AV_LOG_VERBOSE, "Successfully set duplex mode to %s duplex.\n", 
ctx->duplex_mode == 2  || ctx->duplex_mode == 4 ? "full" : "half");
+#else
av_log(avctx, AV_LOG_VERBOSE, "Successfully set duplex mode to %s duplex.\n", 
ctx->duplex_mode == 2 ? "full" : "half");
+#endif
} else {
av_log(avctx, AV_LOG_WARNING, "Unable to set duplex mode, because it is 
not supported.\n");
}
diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
index ad8b33c..a2d6509 100644
--- 

Re: [FFmpeg-devel] [PATCH v2] avcodec/mpeg12dec: report error when picture type is unknown and err_detect is EXPLODE

2021-08-07 Thread Michael Niedermayer
On Sat, Aug 07, 2021 at 09:21:36AM +0200, Marton Balint wrote:
> 
> 
> On Fri, 6 Aug 2021, Michael Niedermayer wrote:
> 
> > On Mon, Aug 02, 2021 at 08:50:05PM +0200, Marton Balint wrote:
> > > Also split error message to error and warning.
> > > 
> > > Signed-off-by: Marton Balint 
> > > ---
> > >  libavcodec/mpeg12dec.c | 14 ++
> > >  1 file changed, 10 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
> > > index 858dca660c..49d865853b 100644
> > > --- a/libavcodec/mpeg12dec.c
> > > +++ b/libavcodec/mpeg12dec.c
> > > @@ -1529,7 +1529,7 @@ static void 
> > > mpeg_decode_quant_matrix_extension(MpegEncContext *s)
> > >  load_matrix(s, s->chroma_inter_matrix, NULL, 0);
> > >  }
> > > 
> > > -static void mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
> > > +static int mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
> > >  {
> > >  MpegEncContext *s = >mpeg_enc_ctx;
> > > 
> > > @@ -1539,8 +1539,10 @@ static void 
> > > mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
> > >  s->mpeg_f_code[1][0] = get_bits(>gb, 4);
> > >  s->mpeg_f_code[1][1] = get_bits(>gb, 4);
> > >  if (!s->pict_type && s1->mpeg_enc_ctx_allocated) {
> > > -av_log(s->avctx, AV_LOG_ERROR,
> > > -   "Missing picture start code, guessing missing values\n");
> > > +av_log(s->avctx, AV_LOG_ERROR, "Missing picture start code\n");
> > > +if (s->avctx->err_recognition & AV_EF_EXPLODE)
> > > +return AVERROR_INVALIDDATA;
> > > +av_log(s->avctx, AV_LOG_WARNING, "Guessing pict_type from 
> > > mpeg_f_code\n");
> > 
> > If we are nitpicking then this is not ideal
> > "Missing picture start code" is only an error when AV_EF_EXPLODE is set
> > because it only in that case results in wrong output, when AV_EF_EXPLODE
> > is not set the output from the decoder might be correct si it too should
> > be a warning in that case
> 
> I understand your logic, but there are arguments for keeping the error
> message as well. I think in practice, most times, this error is caused by
> bitstream errors and it _will_ affect decoding. 

> Also there are tons of cases
> in the code which log an error, then return INVALIDDATA if EXPLODE is used,
> so why handle it here differently...

do these cases really represent situations where the output might be correct ?
for examlple any out of range coefficient cant be correct and a mismatching CRC
well it "could" be.
I think a big part would be, "do we actually have cases where it is not an
error ?" if so i would argue that it should be a warning

> 
> But let me know if you insisit and I will change it. And yes, it is indeed
> nitpicking :)

i dont insist but thats how the levels are defined

/**
 * Something went wrong and cannot losslessly be recovered.
 * However, not all future data is affected.
 */
#define AV_LOG_ERROR16

/**
 * Something somehow does not look correct. This may or may not
 * lead to problems. An example would be the use of '-vstrict -2'.
 */
#define AV_LOG_WARNING  24


[...]

-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

"I am not trying to be anyone's saviour, I'm trying to think about the
 future and not be sad" - Elon Musk



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2 3/4] avdevice/decklink: add levelA configure

2021-08-07 Thread Marton Balint




On Fri, 6 Aug 2021, lance.lmw...@gmail.com wrote:


From: Limin Wang 

Signed-off-by: Limin Wang 
---
just rebase the code to the master branch to fix the first warning.

doc/outdevs.texi|  4 
libavdevice/decklink_common.cpp | 17 +
libavdevice/decklink_common_c.h |  1 +
libavdevice/decklink_enc_c.c|  1 +
4 files changed, 23 insertions(+)

diff --git a/doc/outdevs.texi b/doc/outdevs.texi
index c4c1eba..dee9de3 100644
--- a/doc/outdevs.texi
+++ b/doc/outdevs.texi
@@ -214,6 +214,10 @@ Defaults to @samp{unset}.
If set to @option{true}, Quad-link SDI is output in Square Division Quad Split 
mode.
Defaults to @option{false}.

+@item levelA


level_a, no camelcase for option names.


+If set to @option{true}, SMPTE Level A is enable on the used output.


If set to @option{true}, SMPTE Level A is enabled on the SDI output.


+Defaults to @option{false}.


Same here, please add a default (-1) unset mode.


+
@end table

@subsection Examples
diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
index bb69a54..46e9768 100644
--- a/libavdevice/decklink_common.cpp
+++ b/libavdevice/decklink_common.cpp
@@ -234,6 +234,23 @@ int ff_decklink_set_configs(AVFormatContext *avctx,
}
}

+if (direction == DIRECTION_OUT && cctx->levelA) {
+DECKLINK_BOOL levelA_supported = false;


level_a_supported


+
+if (ctx->attr->GetFlag(BMDDeckLinkSupportsSMPTELevelAOutput, 
_supported) != S_OK)
+levelA_supported = false;
+
+if (levelA_supported) {
+res = ctx->cfg->SetFlag(bmdDeckLinkConfigSMPTELevelAOutput, 
cctx->levelA);
+if (res != S_OK)
+av_log(avctx, AV_LOG_WARNING, "Setting SMPTE levelA 
failed.\n");
+else
+av_log(avctx, AV_LOG_VERBOSE, "Successfully set SMPTE 
levelA.\n");
+} else {
+av_log(avctx, AV_LOG_WARNING, "Unable to set SMPTE levelA mode, because 
it is not supported.\n");
+}
+}
+
return 0;
}

diff --git a/libavdevice/decklink_common_c.h b/libavdevice/decklink_common_c.h
index fdaa1f9..d855311 100644
--- a/libavdevice/decklink_common_c.h
+++ b/libavdevice/decklink_common_c.h
@@ -50,6 +50,7 @@ struct decklink_cctx {
int duplex_mode;
int link;
int sqd;
+int levelA;


level_a


DecklinkPtsSource audio_pts_source;
DecklinkPtsSource video_pts_source;
int audio_input;
diff --git a/libavdevice/decklink_enc_c.c b/libavdevice/decklink_enc_c.c
index b26c93b..614a84a 100644
--- a/libavdevice/decklink_enc_c.c
+++ b/libavdevice/decklink_enc_c.c
@@ -40,6 +40,7 @@ static const AVOption options[] = {
{ "single"  ,  NULL , 0   , AV_OPT_TYPE_CONST 
, { .i64 = 1   }, 0, 0, ENC, "link"},
{ "dual",  NULL , 0   , AV_OPT_TYPE_CONST 
, { .i64 = 2   }, 0, 0, ENC, "link"},
{ "quad",  NULL , 0   , AV_OPT_TYPE_CONST 
, { .i64 = 3   }, 0, 0, ENC, "link"},
+{ "levelA"  , "set SMPTE LevelA", OFFSET(levelA)  , 
AV_OPT_TYPE_BOOL,   { .i64 = 0   }, 0, 1, ENC },


"level_a"


{ "sqd" , "set Square Division" , OFFSET(sqd) , 
AV_OPT_TYPE_BOOL,   { .i64 = 0   }, 0, 1, ENC },
{ "timing_offset", "genlock timing pixel offset", OFFSET(timing_offset), 
AV_OPT_TYPE_INT,   { .i64 = INT_MIN }, INT_MIN, INT_MAX, ENC, "timing_offset"},
{ "unset"   ,  NULL , 0, 
AV_OPT_TYPE_CONST, { .i64 = INT_MIN },   0,   0, ENC, "timing_offset"},
--
1.8.3.1


Thanks,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2 2/4] avdevice/decklink: add sqd configure

2021-08-07 Thread Marton Balint




On Fri, 6 Aug 2021, lance.lmw...@gmail.com wrote:


From: Limin Wang 

Signed-off-by: Limin Wang 
---
doc/outdevs.texi|  4 
libavdevice/decklink_common.cpp | 11 +++
libavdevice/decklink_common_c.h |  1 +
libavdevice/decklink_enc_c.c|  1 +
4 files changed, 17 insertions(+)

diff --git a/doc/outdevs.texi b/doc/outdevs.texi
index dd55904..c4c1eba 100644
--- a/doc/outdevs.texi
+++ b/doc/outdevs.texi
@@ -210,6 +210,10 @@ Sets the video link configuration on the used output. Must 
be @samp{unset}, @sam
@samp{dual}, @samp{quad}.
Defaults to @samp{unset}.

+@item sqd
+If set to @option{true}, Quad-link SDI is output in Square Division Quad Split 
mode.
+Defaults to @option{false}.


Please add an unset mode (-1) for this option and make that the default. 
User may not want to override the default configuration.



+
@end table

@subsection Examples
diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
index d7b4829..bb69a54 100644
--- a/libavdevice/decklink_common.cpp
+++ b/libavdevice/decklink_common.cpp
@@ -221,6 +221,17 @@ int ff_decklink_set_configs(AVFormatContext *avctx,
av_log(avctx, AV_LOG_WARNING, "Setting link configuration 
failed.\n");
else
av_log(avctx, AV_LOG_VERBOSE, "Successfully set link configuration: 
0x%x.\n", ctx->link);
+if (ctx->link == bmdLinkConfigurationQuadLink && cctx->sqd) {
+#if BLACKMAGIC_DECKLINK_API_VERSION >= 0x0a0b0400
+res = 
ctx->cfg->SetFlag(bmdDeckLinkConfigQuadLinkSDIVideoOutputSquareDivisionSplit, 
cctx->sqd);
+if (res != S_OK)
+av_log(avctx, AV_LOG_WARNING, "Setting SquareDivisionSplit 
failed.\n");
+else
+av_log(avctx, AV_LOG_VERBOSE, "Successfully set 
SquareDivisionSplit.\n");
+#else
+av_log(avctx, AV_LOG_VERBOSE, "Unable to set SquareDivisionSplit, require 
version of SDK  >= 10.11.4.\n");


Just bump the SDK version requirement in configure, 10.11.4 is only 
slightly older than 10.10, and there is no reason to support ancient 
versions forever.


Thanks,
Marton


+#endif
+}
}

return 0;
diff --git a/libavdevice/decklink_common_c.h b/libavdevice/decklink_common_c.h
index f37e0c0..fdaa1f9 100644
--- a/libavdevice/decklink_common_c.h
+++ b/libavdevice/decklink_common_c.h
@@ -49,6 +49,7 @@ struct decklink_cctx {
int audio_depth;
int duplex_mode;
int link;
+int sqd;
DecklinkPtsSource audio_pts_source;
DecklinkPtsSource video_pts_source;
int audio_input;
diff --git a/libavdevice/decklink_enc_c.c b/libavdevice/decklink_enc_c.c
index d85d540..b26c93b 100644
--- a/libavdevice/decklink_enc_c.c
+++ b/libavdevice/decklink_enc_c.c
@@ -40,6 +40,7 @@ static const AVOption options[] = {
{ "single"  ,  NULL , 0   , AV_OPT_TYPE_CONST 
, { .i64 = 1   }, 0, 0, ENC, "link"},
{ "dual",  NULL , 0   , AV_OPT_TYPE_CONST 
, { .i64 = 2   }, 0, 0, ENC, "link"},
{ "quad",  NULL , 0   , AV_OPT_TYPE_CONST 
, { .i64 = 3   }, 0, 0, ENC, "link"},
+{ "sqd" , "set Square Division" , OFFSET(sqd) , 
AV_OPT_TYPE_BOOL,   { .i64 = 0   }, 0, 1, ENC },
{ "timing_offset", "genlock timing pixel offset", OFFSET(timing_offset), 
AV_OPT_TYPE_INT,   { .i64 = INT_MIN }, INT_MIN, INT_MAX, ENC, "timing_offset"},
{ "unset"   ,  NULL , 0, 
AV_OPT_TYPE_CONST, { .i64 = INT_MIN },   0,   0, ENC, "timing_offset"},
{ NULL },
--
1.8.3.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-requ...@ffmpeg.org with subject "unsubscribe".


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2 1/4] avdevice/decklink: add link configuration

2021-08-07 Thread Marton Balint




On Fri, 6 Aug 2021, lance.lmw...@gmail.com wrote:


From: Limin Wang 

Signed-off-by: Limin Wang 
---
doc/outdevs.texi| 5 +
libavdevice/decklink_common.cpp | 9 +
libavdevice/decklink_common.h   | 8 
libavdevice/decklink_common_c.h | 1 +
libavdevice/decklink_enc.cpp| 2 ++
libavdevice/decklink_enc_c.c| 5 +
6 files changed, 30 insertions(+)

diff --git a/doc/outdevs.texi b/doc/outdevs.texi
index aaf2479..dd55904 100644
--- a/doc/outdevs.texi
+++ b/doc/outdevs.texi
@@ -205,6 +205,11 @@ Defaults to @samp{unset}.
Sets the genlock timing pixel offset on the used output.
Defaults to @samp{unset}.

+@item link
+Sets the video link configuration on the used output. Must be @samp{unset}, 
@samp{single},
+@samp{dual}, @samp{quad}.


Sets the SDI video link configuration on the used output. Must be 
@samp{unset}, @samp{single} link SDI, @samp{dual} link SDI or @samp{quad} 
link SDI.



+Defaults to @samp{unset}.
+
@end table

@subsection Examples
diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp
index 24aa9b1..d7b4829 100644
--- a/libavdevice/decklink_common.cpp
+++ b/libavdevice/decklink_common.cpp
@@ -214,6 +214,15 @@ int ff_decklink_set_configs(AVFormatContext *avctx,
if (res != S_OK)
av_log(avctx, AV_LOG_WARNING, "Setting timing offset failed.\n");
}
+
+if (direction == DIRECTION_OUT && ctx->link > 0 ) {


stray whitespace before )


+res = ctx->cfg->SetInt(bmdDeckLinkConfigSDIOutputLinkConfiguration, 
ctx->link);
+if (res != S_OK)
+av_log(avctx, AV_LOG_WARNING, "Setting link configuration 
failed.\n");
+else
+av_log(avctx, AV_LOG_VERBOSE, "Successfully set link configuration: 
0x%x.\n", ctx->link);
+}
+
return 0;
}

diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h
index 6e03295..ad8b33c 100644
--- a/libavdevice/decklink_common.h
+++ b/libavdevice/decklink_common.h
@@ -131,6 +131,7 @@ struct decklink_ctx {
int64_t teletext_lines;
double preroll;
int duplex_mode;
+BMDLinkConfiguration link;
DecklinkPtsSource audio_pts_source;
DecklinkPtsSource video_pts_source;
int draw_bars;
@@ -200,6 +201,13 @@ static const BMDTimecodeFormat 
decklink_timecode_format_map[] = {
#endif
};

+static const BMDLinkConfiguration decklink_link_conf_map[] = {
+(BMDLinkConfiguration)0,
+bmdLinkConfigurationSingleLink,
+bmdLinkConfigurationDualLink,
+bmdLinkConfigurationQuadLink
+};
+
int ff_decklink_set_configs(AVFormatContext *avctx, decklink_direction_t 
direction);
int ff_decklink_set_format(AVFormatContext *avctx, int width, int height, int 
tb_num, int tb_den, enum AVFieldOrder field_order, decklink_direction_t 
direction = DIRECTION_OUT);
int ff_decklink_set_format(AVFormatContext *avctx, decklink_direction_t 
direction);
diff --git a/libavdevice/decklink_common_c.h b/libavdevice/decklink_common_c.h
index 68978fa..f37e0c0 100644
--- a/libavdevice/decklink_common_c.h
+++ b/libavdevice/decklink_common_c.h
@@ -48,6 +48,7 @@ struct decklink_cctx {
int audio_channels;
int audio_depth;
int duplex_mode;
+int link;
DecklinkPtsSource audio_pts_source;
DecklinkPtsSource video_pts_source;
int audio_input;
diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp
index 4c1eb05..6dec5f3 100644
--- a/libavdevice/decklink_enc.cpp
+++ b/libavdevice/decklink_enc.cpp
@@ -559,6 +559,8 @@ av_cold int ff_decklink_write_header(AVFormatContext *avctx)
ctx->list_formats = cctx->list_formats;
ctx->preroll  = cctx->preroll;
ctx->duplex_mode  = cctx->duplex_mode;
+if (cctx->link > 0 && (unsigned int)cctx->link < 
FF_ARRAY_ELEMS(decklink_link_conf_map))
+ctx->link = decklink_link_conf_map[cctx->link];
cctx->ctx = ctx;
#if CONFIG_LIBKLVANC
if (klvanc_context_create(>vanc_ctx) < 0) {
diff --git a/libavdevice/decklink_enc_c.c b/libavdevice/decklink_enc_c.c
index 828cf5d..d85d540 100644
--- a/libavdevice/decklink_enc_c.c
+++ b/libavdevice/decklink_enc_c.c
@@ -35,6 +35,11 @@ static const AVOption options[] = {
{ "unset"   ,  NULL , 0   , AV_OPT_TYPE_CONST 
, { .i64 = 0   }, 0, 0, ENC, "duplex_mode"},
{ "half",  NULL , 0   , AV_OPT_TYPE_CONST 
, { .i64 = 1   }, 0, 0, ENC, "duplex_mode"},
{ "full",  NULL , 0   , AV_OPT_TYPE_CONST 
, { .i64 = 2   }, 0, 0, ENC, "duplex_mode"},
+{ "link" , "link configure" , OFFSET(link), AV_OPT_TYPE_INT  
 , { .i64 = 0   }, 0, 3, ENC, "link"},


"single/dual/quad SDI link configuration"


+{ "unset"   ,  NULL , 0   , 
AV_OPT_TYPE_CONST , { .i64 = 0   }, 0, 0, ENC, "link"},
+{ "single"  ,  NULL , 0   , 
AV_OPT_TYPE_CONST , { .i64 = 1   }, 0, 0, ENC, "link"},
+{ "dual"

Re: [FFmpeg-devel] [PATCH] fftools/cmdutils: don't print build configuration by default

2021-08-07 Thread James Almer

On 8/7/2021 1:17 PM, Derek Buitenhuis wrote:

On 8/7/2021 5:13 PM, James Almer wrote:

It's just to reduce the amount of information printed by default. Most
command line utilities don't bother showing build time configuration
options outside of --version or --help output, if at all.
And the string is obviously in the binary (duplicated on every library
at that), so it's not about hiding anything.


I'm going to have to say this is actually negative value, and say I dislike it.

I find it quite useful to print by default, in terms of debugging, finding
issues in logs, etc.

It also means (as noted elsewhere) that bug reports no longer include this
info by default.


Bug reports are usually required to use debug loglevel, so they should 
not be affected with the latest version.




So, I'm going to have to say this is not a good enough reason, in my opinion.


Alright.




I made the latest version print it on verbose level or higher (and
always with --version), which may be preferable. But i nonetheless don't
have a strong opinion about this change.


See above.

- Derek

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] fftools/cmdutils: don't print build configuration by default

2021-08-07 Thread Derek Buitenhuis
On 8/7/2021 5:13 PM, James Almer wrote:
> It's just to reduce the amount of information printed by default. Most 
> command line utilities don't bother showing build time configuration 
> options outside of --version or --help output, if at all.
> And the string is obviously in the binary (duplicated on every library 
> at that), so it's not about hiding anything.

I'm going to have to say this is actually negative value, and say I dislike it.

I find it quite useful to print by default, in terms of debugging, finding
issues in logs, etc.

It also means (as noted elsewhere) that bug reports no longer include this
info by default.

So, I'm going to have to say this is not a good enough reason, in my opinion.

> I made the latest version print it on verbose level or higher (and 
> always with --version), which may be preferable. But i nonetheless don't 
> have a strong opinion about this change.

See above.

- Derek

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] fftools/cmdutils: don't print build configuration by default

2021-08-07 Thread James Almer

On 8/7/2021 1:06 PM, Derek Buitenhuis wrote:

On 8/6/2021 7:04 PM, James Almer wrote:

From: Matthieu Patou 

Suggested-by: ffm...@fb.com
Signed-off-by: James Almer 
---
  fftools/cmdutils.c | 5 +++--
  1 file changed, 3 insertions(+), 2 deletions(-)


What exactly is the point of this?

You've provided a patch with no explanation for its value.

If it's "security", then that's kind of dumb, since if you
have access to run ffmpeg on something, you have access to
`strings $(which ffmpeg)` too.


It's just to reduce the amount of information printed by default. Most 
command line utilities don't bother showing build time configuration 
options outside of --version or --help output, if at all.
And the string is obviously in the binary (duplicated on every library 
at that), so it's not about hiding anything.


I made the latest version print it on verbose level or higher (and 
always with --version), which may be preferable. But i nonetheless don't 
have a strong opinion about this change.




- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".



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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] fftools/cmdutils: don't print build configuration by default

2021-08-07 Thread Derek Buitenhuis
On 8/6/2021 7:04 PM, James Almer wrote:
> From: Matthieu Patou 
> 
> Suggested-by: ffm...@fb.com
> Signed-off-by: James Almer 
> ---
>  fftools/cmdutils.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

What exactly is the point of this?

You've provided a patch with no explanation for its value.

If it's "security", then that's kind of dumb, since if you
have access to run ffmpeg on something, you have access to
`strings $(which ffmpeg)` too.

- Derek
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 3/3] libavfilter/graphparser: Add scale_sws_opts parse support in avfilter_graph_parse2

2021-08-07 Thread Andreas Rheinhardt
Linjie Fu:
> From: Linjie Fu 
> 
> To pass the swscale options for the inserted scalers.
> 
> ffmpeg -i input.mp4 -filter_complex \
> "scale_sws_opts=alphablend=checkerboard;format=nv12" \
> -t 0.5 output.mp4
> 
> Update docs.
> 
> Signed-off-by: Linjie Fu 
> ---
>  doc/filters.texi  |  7 ---
>  libavfilter/graphparser.c | 27 +++
>  2 files changed, 31 insertions(+), 3 deletions(-)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 790d165433..dbbb3a6940 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -204,9 +204,9 @@ pads must be connected. A filtergraph is considered valid 
> if all the
>  filter input and output pads of all the filterchains are connected.
>  
>  Libavfilter will automatically insert @ref{scale} filters where format
> -conversion is required. It is possible to specify swscale flags
> -for those automatically inserted scalers by prepending
> -@code{sws_flags=@var{flags};}
> +conversion is required. It is possible to specify swscale flags or
> +scale_sws_opts for those automatically inserted scalers by prepending
> +@code{sws_flags=@var{flags};} or @code{scale_sws_opts=@var{scale_sws_opts};}
>  to the filtergraph description.
>  
>  Here is a BNF description of the filtergraph syntax:
> @@ -219,6 +219,7 @@ Here is a BNF description of the filtergraph syntax:
>  @var{FILTER}   ::= [@var{LINKLABELS}] @var{FILTER_NAME} ["=" 
> @var{FILTER_ARGUMENTS}] [@var{LINKLABELS}]
>  @var{FILTERCHAIN}  ::= @var{FILTER} [,@var{FILTERCHAIN}]
>  @var{FILTERGRAPH}  ::= [sws_flags=@var{flags};] @var{FILTERCHAIN} 
> [;@var{FILTERGRAPH}]
> +@var{FILTERGRAPH}  ::= [scale_sws_opts=@var{opts};] @var{FILTERCHAIN} 
> [;@var{FILTERGRAPH}]
>  @end example
>  
>  @anchor{filtergraph escaping}
> diff --git a/libavfilter/graphparser.c b/libavfilter/graphparser.c
> index 1385c3ae71..8c63d6454e 100644
> --- a/libavfilter/graphparser.c
> +++ b/libavfilter/graphparser.c
> @@ -415,6 +415,30 @@ static int parse_sws_flags(const char **buf, 
> AVFilterGraph *graph)
>  return 0;
>  }
>  
> +static int parse_scale_sws_opts(const char **buf, AVFilterGraph *graph)
> +{
> +char *p = strchr(*buf, ';');
> +
> +if (strncmp(*buf, "scale_sws_opts=", 15)) {

av_strstart()

> +return 0;
> +}
> +
> +if (!p) {
> +av_log(graph, AV_LOG_ERROR, "scale_sws_opts not terminated with 
> ';'.\n");
> +return AVERROR(EINVAL);
> +}
> +
> +*buf += 15;
> +
> +av_freep(>scale_sws_opts);
> +if (!(graph->scale_sws_opts = av_mallocz(p - *buf + 1)))
> +return AVERROR(ENOMEM);
> +av_strlcpy(graph->scale_sws_opts, *buf, p - *buf + 1);

av_strndup()

> +
> +*buf = p + 1;
> +return 0;
> +}
> +
>  int avfilter_graph_parse2(AVFilterGraph *graph, const char *filters,
>AVFilterInOut **inputs,
>AVFilterInOut **outputs)
> @@ -429,6 +453,9 @@ int avfilter_graph_parse2(AVFilterGraph *graph, const 
> char *filters,
>  if ((ret = parse_sws_flags(, graph)) < 0)
>  goto fail;
>  
> +if ((ret = parse_scale_sws_opts(, graph)) < 0)
> +goto fail;
> +
>  do {
>  AVFilterContext *filter;
>  filters += strspn(filters, WHITESPACES);
> 

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH 2/3] lavfi/vf_scale: dump the exact swscale_options to passed to libswscale

2021-08-07 Thread Andreas Rheinhardt
Linjie Fu:
> From: Linjie Fu 
> 
> Printed verbose log doesn't match the sws_flags specified in the cmdline
> for simple filter graph.
> 
>   ffmpeg .. -sws_flags bicubic ..
>  [auto_scaler_0] w:iw h:ih flags:'' interl:0
>  [auto_scaler_0] w:310 h:449 fmt:yuva420p sar:0/1 -> w:310 h:449 fmt:yuv420p 
> sar:0/1 flags:0x0
> 
> Filter complex doesn't have this issue as mentioned in 12e7e1d03, the
> auto-inserted scaler accepts sws_flags in filtergraph complex which
> overrides the 'flags' option for vf_scale and dump it as a verbose log:
> 
>   ffmpeg ..  -filter_complex "sws_flags=bicubic;format=nv12" ..
>  [auto_scaler_0] w:iw h:ih flags:'bicubic' interl:0
>  [auto_scaler_0] w:310 h:449 fmt:yuva420p sar:0/1 -> w:310 h:449 fmt:nv12 
> sar:0/1 flags:0x2
> 
> To catch the difference, dump the exact sws_flags which is passed to
> libswscale.
> 
>  [auto_scaler_0] swscale_options:'sws_flags=bicubic'
> 
> Signed-off-by: Linjie Fu 
> ---
>  libavfilter/vf_scale.c | 6 ++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index aa855b894a..f994217bdc 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -552,11 +552,17 @@ static int config_props(AVFilterLink *outlink)
> scale->out_range == AVCOL_RANGE_JPEG, 0);
>  
>  if (scale->opts) {
> +char args[512];
> +args[0] = 0;

This will lead to a declaration-after-statement warning.

>  AVDictionaryEntry *e = NULL;
>  while ((e = av_dict_get(scale->opts, "", e, 
> AV_DICT_IGNORE_SUFFIX))) {
>  if ((ret = av_opt_set(*s, e->key, e->value, 0)) < 0)
>  return ret;
> +av_strlcatf(args, sizeof(args), "%s=%s:", e->key, 
> e->value);
>  }
> +if (strlen(args))

I doubt strlen(args) == 0 can happen; anyway, checking for whether a
string is empty can be done easier: "if (args[0] != '\0')".

> +args[strlen(args)-1] = 0;
> +av_log(ctx, AV_LOG_VERBOSE, "swscale_options:'%s'\n", (char 
> *)av_x_if_null(args, ""));

The av_x_if_null() makes no sense at all, as args is never NULL: It is a
stack array.

>  }
>  /* Override YUV420P default settings to have the correct 
> (MPEG-2) chroma positions
>   * MPEG-2 chroma positions are used by convention
> 

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2] doc/developer: Add description about safely sending patches via E-Mail clients

2021-08-07 Thread Andreas Rheinhardt
Soft Works:
> (v2: fix doc build)

This should be directly after the --- below.

> 
> Signed-off-by: softworkz 
> ---
>  doc/developer.texi | 16 
>  1 file changed, 16 insertions(+)
> 
> diff --git a/doc/developer.texi b/doc/developer.texi
> index b33cab0fc7..ce5500b85d 100644
> --- a/doc/developer.texi
> +++ b/doc/developer.texi
> @@ -494,6 +494,22 @@ patch is inline or attached per mail.
>  You can check @url{https://patchwork.ffmpeg.org}, if your patch does not 
> show up, its mime type
>  likely was wrong.
>  
> +@subheading Sending E-Mail Client - Example for MS Outlook.
> +Using @code{git send-email} might not be desirable for everyone. The 
> +following trick allows to send patches via E-Mail clients in a safe 
> +way. It has been tested with Outlook and Thunderbird (with X-Unsent
> +extension) and might work with other applications.
> +
> +Create your patch like this:
> +
> +@verbatim
> +git format-patch -s -o "outputfolder" --add-header "X-Unsent: 1" --suffix 
> .eml --to ffmpeg-devel@ffmpeg.org -1 1a2b3c4d
> +@end verbatim
> +
> +Now you'll just need to open the eml file with the E-Mail application
> +and execure 'Send'.

I guess this is supposed to be execute.

> +
> +@subheading Reviews.
>  Your patch will be reviewed on the mailing list. You will likely be asked
>  to make some changes and are expected to send in an improved version that
>  incorporates the requests from the review. This process may go through
> 

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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [FFmpeg-cvslog] fftools/ffmpeg_filter: fix the flags parsing for scaler

2021-08-07 Thread Linjie Fu
On Sat, Aug 7, 2021 at 2:33 AM Michael Niedermayer 
wrote:

> On Thu, Aug 05, 2021 at 02:30:32PM +, Linjie Fu wrote:
> > ffmpeg | branch: master | Linjie Fu  | Sun
> Aug  1 18:58:19 2021 +0800| [b3a0548a981db52911dd34d9de254c4fee0a8f79] |
> committer: Linjie Fu
> >
> > fftools/ffmpeg_filter: fix the flags parsing for scaler
> >
> > Scaler relys on "-sws_flags" option to pass the flags to swscale
> > and scale filter. Currently passing "sws_flags=xxx" as a filter
> > option to scaler leads to an incorrect option parsing.
> >
> > Check and change the string to "flags=xxx" and dumped flags information.
> > (Refer to parse_sws_flags())
> >
> > CMD:
> > $ffmpeg -v verbose -i input.mp4 -sws_flags lanczos+bitexact+accurate_rnd
> \
> > -vf format=yuv420p,scale=800x600 -an -vframes 10 -f md5 -
> >
> > Before:
> > [auto_scaler_0 @ 0x7f96c3808680] w:iw h:ih flags:'' interl:0
> > [auto_scaler_0 @ 0x7f96c3808680] w:1920 h:1080 fmt:yuvj420p sar:0/1 ->
> w:1920 h:1080 fmt:yuv420p sar:0/1 flags:0x0
> > [Parsed_scale_1 @ 0x7f96c3806e40] w:1920 h:1080 fmt:yuv420p sar:0/1 ->
> w:800 h:600 fmt:yuv420p sar:0/1 flags:0x0
> > MD5=ff1d6091690c6fcd36d458d2a9f648ce
> >
> > After:
> > [auto_scaler_0 @ 0x7fe94563b4c0] w:iw h:ih
> flags:'lanczos+bitexact+accurate_rnd' interl:0
> > [auto_scaler_0 @ 0x7fe94563b4c0] w:1920 h:1080 fmt:yuvj420p sar:0/1 ->
> w:1920 h:1080 fmt:yuv420p sar:0/1 flags:0xc0200
> > [Parsed_scale_1 @ 0x7fe945639d00] w:1920 h:1080 fmt:yuv420p sar:0/1 ->
> w:800 h:600 fmt:yuv420p sar:0/1 flags:0xc0200
> > MD5=ff1d6091690c6fcd36d458d2a9f648ce
> >
> > Signed-off-by: Linjie Fu 
> >
> > >
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=b3a0548a981db52911dd34d9de254c4fee0a8f79
> > ---
>
> This breaks
> -i laraShadow_dl.flv  -alphablend checkerboard -qscale 2 -t 0.5 file.avi
>

Thanks. Double checked, this commit should be reverted since options are
parsed and passed to libswscale correctly.
Previous issue I was trying to fix is more like a log print issue rather
than an option parsing issue.
New patch sent to the maillist. Sorry for the revert.

After this commit the output is just black
>
> sample seems here:
> https://samples.ffmpeg.org/FLV/flash_with_alpha/


Does filter_complex support the same usage or there are other methods to
pass  swscale_option?

# ffmpeg -i laraShadow_dl.flv -filter_complex
"sws_flags=bilinear;format=yuv420p" -alphablend checkerboard -y patched.avi

The output is black,  "-alphablend checkerboard" seems not passed to the
auto-inserted scale filter.

- linjie
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 3/3] libavfilter/graphparser: Add scale_sws_opts parse support in avfilter_graph_parse2

2021-08-07 Thread Linjie Fu
From: Linjie Fu 

To pass the swscale options for the inserted scalers.

ffmpeg -i input.mp4 -filter_complex \
"scale_sws_opts=alphablend=checkerboard;format=nv12" \
-t 0.5 output.mp4

Update docs.

Signed-off-by: Linjie Fu 
---
 doc/filters.texi  |  7 ---
 libavfilter/graphparser.c | 27 +++
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/doc/filters.texi b/doc/filters.texi
index 790d165433..dbbb3a6940 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -204,9 +204,9 @@ pads must be connected. A filtergraph is considered valid 
if all the
 filter input and output pads of all the filterchains are connected.
 
 Libavfilter will automatically insert @ref{scale} filters where format
-conversion is required. It is possible to specify swscale flags
-for those automatically inserted scalers by prepending
-@code{sws_flags=@var{flags};}
+conversion is required. It is possible to specify swscale flags or
+scale_sws_opts for those automatically inserted scalers by prepending
+@code{sws_flags=@var{flags};} or @code{scale_sws_opts=@var{scale_sws_opts};}
 to the filtergraph description.
 
 Here is a BNF description of the filtergraph syntax:
@@ -219,6 +219,7 @@ Here is a BNF description of the filtergraph syntax:
 @var{FILTER}   ::= [@var{LINKLABELS}] @var{FILTER_NAME} ["=" 
@var{FILTER_ARGUMENTS}] [@var{LINKLABELS}]
 @var{FILTERCHAIN}  ::= @var{FILTER} [,@var{FILTERCHAIN}]
 @var{FILTERGRAPH}  ::= [sws_flags=@var{flags};] @var{FILTERCHAIN} 
[;@var{FILTERGRAPH}]
+@var{FILTERGRAPH}  ::= [scale_sws_opts=@var{opts};] @var{FILTERCHAIN} 
[;@var{FILTERGRAPH}]
 @end example
 
 @anchor{filtergraph escaping}
diff --git a/libavfilter/graphparser.c b/libavfilter/graphparser.c
index 1385c3ae71..8c63d6454e 100644
--- a/libavfilter/graphparser.c
+++ b/libavfilter/graphparser.c
@@ -415,6 +415,30 @@ static int parse_sws_flags(const char **buf, AVFilterGraph 
*graph)
 return 0;
 }
 
+static int parse_scale_sws_opts(const char **buf, AVFilterGraph *graph)
+{
+char *p = strchr(*buf, ';');
+
+if (strncmp(*buf, "scale_sws_opts=", 15)) {
+return 0;
+}
+
+if (!p) {
+av_log(graph, AV_LOG_ERROR, "scale_sws_opts not terminated with 
';'.\n");
+return AVERROR(EINVAL);
+}
+
+*buf += 15;
+
+av_freep(>scale_sws_opts);
+if (!(graph->scale_sws_opts = av_mallocz(p - *buf + 1)))
+return AVERROR(ENOMEM);
+av_strlcpy(graph->scale_sws_opts, *buf, p - *buf + 1);
+
+*buf = p + 1;
+return 0;
+}
+
 int avfilter_graph_parse2(AVFilterGraph *graph, const char *filters,
   AVFilterInOut **inputs,
   AVFilterInOut **outputs)
@@ -429,6 +453,9 @@ int avfilter_graph_parse2(AVFilterGraph *graph, const char 
*filters,
 if ((ret = parse_sws_flags(, graph)) < 0)
 goto fail;
 
+if ((ret = parse_scale_sws_opts(, graph)) < 0)
+goto fail;
+
 do {
 AVFilterContext *filter;
 filters += strspn(filters, WHITESPACES);
-- 
2.31.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-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 2/3] lavfi/vf_scale: dump the exact swscale_options to passed to libswscale

2021-08-07 Thread Linjie Fu
From: Linjie Fu 

Printed verbose log doesn't match the sws_flags specified in the cmdline
for simple filter graph.

  ffmpeg .. -sws_flags bicubic ..
 [auto_scaler_0] w:iw h:ih flags:'' interl:0
 [auto_scaler_0] w:310 h:449 fmt:yuva420p sar:0/1 -> w:310 h:449 fmt:yuv420p 
sar:0/1 flags:0x0

Filter complex doesn't have this issue as mentioned in 12e7e1d03, the
auto-inserted scaler accepts sws_flags in filtergraph complex which
overrides the 'flags' option for vf_scale and dump it as a verbose log:

  ffmpeg ..  -filter_complex "sws_flags=bicubic;format=nv12" ..
 [auto_scaler_0] w:iw h:ih flags:'bicubic' interl:0
 [auto_scaler_0] w:310 h:449 fmt:yuva420p sar:0/1 -> w:310 h:449 fmt:nv12 
sar:0/1 flags:0x2

To catch the difference, dump the exact sws_flags which is passed to
libswscale.

 [auto_scaler_0] swscale_options:'sws_flags=bicubic'

Signed-off-by: Linjie Fu 
---
 libavfilter/vf_scale.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index aa855b894a..f994217bdc 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -552,11 +552,17 @@ static int config_props(AVFilterLink *outlink)
scale->out_range == AVCOL_RANGE_JPEG, 0);
 
 if (scale->opts) {
+char args[512];
+args[0] = 0;
 AVDictionaryEntry *e = NULL;
 while ((e = av_dict_get(scale->opts, "", e, 
AV_DICT_IGNORE_SUFFIX))) {
 if ((ret = av_opt_set(*s, e->key, e->value, 0)) < 0)
 return ret;
+av_strlcatf(args, sizeof(args), "%s=%s:", e->key, 
e->value);
 }
+if (strlen(args))
+args[strlen(args)-1] = 0;
+av_log(ctx, AV_LOG_VERBOSE, "swscale_options:'%s'\n", (char 
*)av_x_if_null(args, ""));
 }
 /* Override YUV420P default settings to have the correct (MPEG-2) 
chroma positions
  * MPEG-2 chroma positions are used by convention
-- 
2.31.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-requ...@ffmpeg.org with subject "unsubscribe".


[FFmpeg-devel] [PATCH 1/3] Revert "fftools/ffmpeg_filter: fix the flags parsing for scaler"

2021-08-07 Thread Linjie Fu
From: Linjie Fu 

This reverts commit b3a0548a981db52911dd34d9de254c4fee0a8f79.

This breaks the usage of swscale options, scale_sws_opts should be passed
to auto-inserted scale-filters.

The auto-inserted scaler accepts sws_flags in filtergraph complex which
overrides the 'flags' option for vf_scale and dump it in verbose, however
simple filtergraph doesn't override. Hence we got different verbose logs
printed in vf_scale when we specify -sws_flags in cmdline. It's a matter
of log print rather than option parsing.

Signed-off-by: Linjie Fu 
---
 fftools/ffmpeg_filter.c | 6 +-
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/fftools/ffmpeg_filter.c b/fftools/ffmpeg_filter.c
index 49076f13ee..a90858655d 100644
--- a/fftools/ffmpeg_filter.c
+++ b/fftools/ffmpeg_filter.c
@@ -977,11 +977,7 @@ int configure_filtergraph(FilterGraph *fg)
 }
 if (strlen(args))
 args[strlen(args)-1] = 0;
-
-if (!strncmp(args, "sws_flags=", 10)) {
-// keep the 'flags=' part
-fg->graph->scale_sws_opts = av_strdup(args+4);
-}
+fg->graph->scale_sws_opts = av_strdup(args);
 
 args[0] = 0;
 while ((e = av_dict_get(ost->swr_opts, "", e,
-- 
2.31.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-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] avutils/hwcontext: When deriving a hwdevice, search for existing device in both directions

2021-08-07 Thread Soft Works
> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Hendrik Leppkes
> Sent: Saturday, 7 August 2021 08:52
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] avutils/hwcontext: When deriving a
> hwdevice, search for existing device in both directions
> 
> On Sat, Aug 7, 2021 at 3:46 AM Soft Works  wrote:
> >
> > The test /libavutil/tests/hwdevice checks that when deriving a device
> > from a source device and then deriving back to the type of the source
> > device, the result is matching the original source device, i.e. the
> > derivation mechanism doesn't create a new device in this case.
> >
> > Previously, this test was usually passed, but only due to two
> > different kind of flaws:
> >
> > 1. The test covers only a single level of derivation (and back)
> >
> > It derives device Y from device X and then Y back to the type of X and
> > checks whether the result matches X.
> >
> > What it doesn't check for, are longer chains of derivation like:
> >
> > CUDA1 > OpenCL2 > CUDA3 and then back to OpenCL4
> >
> > In that case, the second derivation returns the first device (CUDA3 ==
> > CUDA1), but when deriving OpenCL4, hwcontext.c was creating a new
> > OpenCL4 context instead of returning OpenCL2, because there was no
> > link from CUDA1 to OpenCL2 (only backwards from OpenCL2 to CUDA1)
> >
> > If the test would check for two levels of derivation, it would have
> > failed.
> >
> > This patch fixes those (yet untested) cases by introducing forward
> > references (derived_device) in addition to the existing back
> > references (source_device).
> >
> 
> I already see one problem here, if you have so many derived cases
> happening, its also feasible to assume one source was used to derive more
> then one device, which this cannot track, and in fact looks like the code
> would just override and leak a reference.
> 
> How common would it be that such complex derived chains happen, when
> you say even this one is untested?

I'm aware of what you are pointing at, but I couldn't easily construct a case
where this would have an impact.

What we need to consider here is that this is about derivation of HW device
contexts - not frame contexts.

In case of frame contexts, we can have pretty long chains of derivation between
decoder, filters [0-n] and encoder, where we always have a separate HW frames
context in-between. For device contexts, it's instead about creating and 
maintaining just a single one for each device. 

>From a viewpoint of functionality, it's probably only a theoretical issue, but 
it might rarely happen that it gets overwritten. 
Even though it might appear to be overkill, but for a perfect solution, I'd need
to maintain an array of derived_devices.

Shall I do it?

softworkz


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

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH v2] avcodec/mpeg12dec: report error when picture type is unknown and err_detect is EXPLODE

2021-08-07 Thread Marton Balint




On Fri, 6 Aug 2021, Michael Niedermayer wrote:


On Mon, Aug 02, 2021 at 08:50:05PM +0200, Marton Balint wrote:

Also split error message to error and warning.

Signed-off-by: Marton Balint 
---
 libavcodec/mpeg12dec.c | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 858dca660c..49d865853b 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -1529,7 +1529,7 @@ static void 
mpeg_decode_quant_matrix_extension(MpegEncContext *s)
 load_matrix(s, s->chroma_inter_matrix, NULL, 0);
 }

-static void mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
+static int mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
 {
 MpegEncContext *s = >mpeg_enc_ctx;

@@ -1539,8 +1539,10 @@ static void 
mpeg_decode_picture_coding_extension(Mpeg1Context *s1)
 s->mpeg_f_code[1][0] = get_bits(>gb, 4);
 s->mpeg_f_code[1][1] = get_bits(>gb, 4);
 if (!s->pict_type && s1->mpeg_enc_ctx_allocated) {
-av_log(s->avctx, AV_LOG_ERROR,
-   "Missing picture start code, guessing missing values\n");
+av_log(s->avctx, AV_LOG_ERROR, "Missing picture start code\n");
+if (s->avctx->err_recognition & AV_EF_EXPLODE)
+return AVERROR_INVALIDDATA;
+av_log(s->avctx, AV_LOG_WARNING, "Guessing pict_type from 
mpeg_f_code\n");


If we are nitpicking then this is not ideal
"Missing picture start code" is only an error when AV_EF_EXPLODE is set
because it only in that case results in wrong output, when AV_EF_EXPLODE
is not set the output from the decoder might be correct si it too should
be a warning in that case


I understand your logic, but there are arguments for keeping the error 
message as well. I think in practice, most times, this error is caused by 
bitstream errors and it _will_ affect decoding. Also there are tons of 
cases in the code which log an error, then return INVALIDDATA if EXPLODE 
is used, so why handle it here differently...


But let me know if you insisit and I will change it. And yes, it is indeed 
nitpicking :)


Regards,
Marton
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".


Re: [FFmpeg-devel] [PATCH] avutils/hwcontext: When deriving a hwdevice, search for existing device in both directions

2021-08-07 Thread Hendrik Leppkes
On Sat, Aug 7, 2021 at 3:46 AM Soft Works  wrote:
>
> The test /libavutil/tests/hwdevice checks that when deriving a device
> from a source device and then deriving back to the type of the source
> device, the result is matching the original source device, i.e. the
> derivation mechanism doesn't create a new device in this case.
>
> Previously, this test was usually passed, but only due to two different
> kind of flaws:
>
> 1. The test covers only a single level of derivation (and back)
>
> It derives device Y from device X and then Y back to the type of X and
> checks whether the result matches X.
>
> What it doesn't check for, are longer chains of derivation like:
>
> CUDA1 > OpenCL2 > CUDA3 and then back to OpenCL4
>
> In that case, the second derivation returns the first device (CUDA3 ==
> CUDA1), but when deriving OpenCL4, hwcontext.c was creating a new
> OpenCL4 context instead of returning OpenCL2, because there was no link
> from CUDA1 to OpenCL2 (only backwards from OpenCL2 to CUDA1)
>
> If the test would check for two levels of derivation, it would have
> failed.
>
> This patch fixes those (yet untested) cases by introducing forward
> references (derived_device) in addition to the existing back references
> (source_device).
>

I already see one problem here, if you have so many derived cases
happening, its also feasible to assume one source was used to derive
more then one device, which this cannot track, and in fact looks like
the code would just override and leak a reference.

How common would it be that such complex derived chains happen, when
you say even this one is untested?

- Hendrik
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".