Re: [FFmpeg-devel] How to correctly handle Matroska timestamps
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
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
==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
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
> -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
> -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
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
> -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
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
> -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
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
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
> -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
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
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
> -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
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
> -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
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
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
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
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
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
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
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()
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
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
> -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
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"
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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"
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
> -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
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
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".