Re: [FFmpeg-devel] [PATCH v2] mpegvideo_parser: check picture_structure for field order

2022-02-05 Thread Andreas Rheinhardt
Tom Yan:
> the top_field_first bit is only used to indicate the field order
> when the picture is a frame picture (which consists of two fields)
> but not when it is a field picture (which consists of one single
> top or bottom field).
> 
> also removing the unnecessary progressive_sequence check (the bit
> is mandated to be 0 if progressive_frame is 0 on any picture in the
> sequence).
> 

Just because something is mandated does not mean that it is so;
spec-incompliant files exist.

> Signed-off-by: Tom Yan 
> ---
>  libavcodec/mpegvideo_parser.c | 15 +--
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
> index c5dc867d24..004ff602f6 100644
> --- a/libavcodec/mpegvideo_parser.c
> +++ b/libavcodec/mpegvideo_parser.c
> @@ -181,6 +181,7 @@ static void 
> mpegvideo_extract_headers(AVCodecParserContext *s,
>  break;
>  case 0x8: /* picture coding extension */
>  if (bytes_left >= 5) {
> +s->picture_structure = buf[2] & 0x3;
>  top_field_first = buf[3] & (1 << 7);
>  repeat_first_field = buf[3] & (1 << 1);
>  progressive_frame = buf[4] & (1 << 7);
> @@ -198,13 +199,15 @@ static void 
> mpegvideo_extract_headers(AVCodecParserContext *s,
>  }
>  }
>  
> -if (!pc->progressive_sequence && !progressive_frame) 
> {
> -if (top_field_first)
> -s->field_order = AV_FIELD_TT;
> -else
> -s->field_order = AV_FIELD_BB;
> -} else
> +if (progressive_frame)
>  s->field_order = AV_FIELD_PROGRESSIVE;
> +else if (top_field_first ||
> + /* top_field_first is mandated to be 0 when
> +the picture is not a frame picture) */
> + s->picture_structure == 
> AV_PICTURE_STRUCTURE_TOP_FIELD)
> +s->field_order = AV_FIELD_TT;
> +else
> +s->field_order = AV_FIELD_BB;
>  }
>  break;
>  }

___
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] mpegvideo_parser: check picture_structure for field order

2022-02-05 Thread Tom Yan
the top_field_first bit is only used to indicate the field order
when the picture is a frame picture (which consists of two fields)
but not when it is a field picture (which consists of one single
top or bottom field).

also removing the unnecessary progressive_sequence check (the bit
is mandated to be 0 if progressive_frame is 0 on any picture in the
sequence).

Signed-off-by: Tom Yan 
---
 libavcodec/mpegvideo_parser.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
index c5dc867d24..004ff602f6 100644
--- a/libavcodec/mpegvideo_parser.c
+++ b/libavcodec/mpegvideo_parser.c
@@ -181,6 +181,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext 
*s,
 break;
 case 0x8: /* picture coding extension */
 if (bytes_left >= 5) {
+s->picture_structure = buf[2] & 0x3;
 top_field_first = buf[3] & (1 << 7);
 repeat_first_field = buf[3] & (1 << 1);
 progressive_frame = buf[4] & (1 << 7);
@@ -198,13 +199,15 @@ static void 
mpegvideo_extract_headers(AVCodecParserContext *s,
 }
 }
 
-if (!pc->progressive_sequence && !progressive_frame) {
-if (top_field_first)
-s->field_order = AV_FIELD_TT;
-else
-s->field_order = AV_FIELD_BB;
-} else
+if (progressive_frame)
 s->field_order = AV_FIELD_PROGRESSIVE;
+else if (top_field_first ||
+ /* top_field_first is mandated to be 0 when
+the picture is not a frame picture) */
+ s->picture_structure == 
AV_PICTURE_STRUCTURE_TOP_FIELD)
+s->field_order = AV_FIELD_TT;
+else
+s->field_order = AV_FIELD_BB;
 }
 break;
 }
-- 
2.35.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 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

2022-02-05 Thread lance . lmwang
On Sat, Feb 05, 2022 at 01:26:18PM -0800, Chad Fraleigh wrote:
> Since any [valid] value over 255 is impossible in the IPv4 protocol (the TTL 
> field is only 8-bits), it should always be capped at 255 (for consistency) or 
> return an invalid value error (the one I would suggest).
> 

zhilizhao have submit a patch to limit the range of ttl from option. Do you want
to return an invalid error here still?


> Despite VLC's reversed comment, using an int seems to be the exception to the 
> rule (i.e. only linux and windows seem to use it [as-documented]), perhaps 
> doing the unsigned char first and using the int as the fallback would be 
> better? It's not really just a BSD thing, unless you also count LWIP and 
> Solaris as BSD. Unless VLC's code history shows them doing it this way at one 
> time and swapping it (but forgetting the comment) to fix a known bug?
> 

I have blamed vlc code and sure the code doing it this way at one 
time(104938796a3). 
For the mismatch of code and comments, I prefer to code always as code were 
build 
and used by all kinds of system which vlc is supported already. 

As for use BSD, I prefer to count LWIP and Solaris into BSD category which using
rule of byte. If you still prefer to add them into comments, I'm OK also. 

> 
> On 2/4/2022 9:28 PM, lance.lmw...@gmail.com wrote:
> > From: Limin Wang 
> > 
> > Suggested by zhilizhao, vlc project has solved the compatibility by
> > the same way, so I borrowed the comments from vlc project.
> > 
> > Fix #ticket9449
> > 
> > Signed-off-by: Limin Wang 
> > ---
> >  libavformat/udp.c | 15 +--
> >  1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/udp.c b/libavformat/udp.c
> > index 3dc79eb..34488d6 100644
> > --- a/libavformat/udp.c
> > +++ b/libavformat/udp.c
> > @@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int 
> > mcastTTL,
> >  {
> >  int protocol, cmd;
> >  
> > +/* There is some confusion in the world whether IP_MULTICAST_TTL
> > + * takes a byte or an int as an argument.
> > + * BSD seems to indicate byte so we are going with that and use
> > + * int as a fallback to be safe */
> >  switch (addr->sa_family) {
> >  #ifdef IP_MULTICAST_TTL
> >  case AF_INET:
> > @@ -183,8 +187,15 @@ static int udp_set_multicast_ttl(int sockfd, int 
> > mcastTTL,
> >  }
> >  
> >  if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 
> > 0) {
> > -ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> > -return ff_neterrno();
> > +/* BSD compatibility */
> > +unsigned char ttl;
> > +
> > +ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt");
> > +ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
> > +if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
> > +ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> > +return ff_neterrno();
> > +}
> >  }
> >  
> >  return 0;
> ___
> 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".

-- 
Thanks,
Limin Wang
___
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/2] avcodec/{ass, webvttdec}: fix handling of backslashes

2022-02-05 Thread Soft Works


> -Original Message-
> From: ffmpeg-devel  On Behalf Of Soft
> Works
> Sent: Sunday, February 6, 2022 2:09 AM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
> 
> 
> 
> > -Original Message-
> > From: ffmpeg-devel  On Behalf Of
> > Oneric
> > Sent: Saturday, February 5, 2022 11:00 PM
> > To: FFmpeg development discussions and patches  > de...@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}:
> fix
> > handling of backslashes
> >
> > On Sat, Feb 05, 2022 at 02:08:48 +, Soft Works wrote:
> > > Let's try to approach this from a different side. Which case is
> > > your [1/2] commit actually supposed to fix?
> >
> > Escape backslashes when converting from WebVTT to not accidentally
> > introduce active ASS sequences and replace the wrong backslash-
> escape
> > in ff_ass_bprint_text_event with an escape that actually works.
> >
> > > How did you test your patch?
> > > Can we please go over an example?
> >
> > Take a look at the attached WebVTT file.
> 
> Thanks a lot for the test file!
> 
> 
> > We expect the second event to be rendered like this,
> > as from WebVTT’s point of view it’s just normal text:
> >
> >   our final \h approach \N into \ Coruscant.
> >
> > What we currently get after conversion to ASS is like this though
> > (pay attention to the number of spaces):
> >
> >   our final   approach
> >   into \ Coruscant.
> 
> Yup, no doubt that this is wrong.
> 
> 
> > If instead the word-joiner is appended as in my patch, the
> > visual output matches the expectation (mail does not contain
> U+2060):
> >
> >   our final \h approach \N into \ Coruscant.
> 
> I can also confirm that your patch would "work" with regards
> to ass output when trying with both "old" libass and new libass.
> I haven't tried with other implementations yet, but when this
> would turn out to be working with all usual implementations,
> I might even be OK with the word-joiner.
> 
> But this is where the agreement ends.
> 
> - If at all, the word-joiner insertion would need to be
>   limited to ASS output ONLY
> - it would need to be controllable through an option in the ASS
>   encoder
> - The word joiners should not be used in internal processing and
>   only be (optionally) added when encoding to ass
> - Unfortunately, the FATE tests for the other subtitle formats
>   do not include these sequences in the test source files, and
>   that means, before making such change that might potentially
>   affect all other text subtitle encoders, those sequences would
>   need to be added to make sure these conversion won't be affected
> - Generally, those changes (also the BIDI mark insertion) should
>   be evaluated with regards to all text subtitle encoders,
>   making sure there's no side effect.
> 
> You said:
> 
> > I’m not interested in reworking ffmpeg’s internal subtitle handling.
> > The proposed patch is a clear improvement over the status quo which
> > is plain incorrect. Within reasonable effort and sound arguments for
> > it adjustments to the patch can be made; reworking ffmpeg internals
> is
> > imo not “reasonable” effort to correct an uncontestedly wrong
> escape.
> 
> And that is a problem. The changes you are proposing are making
> changes to ffmpeg’s internal subtitle handling, so you need to decide
> whether you want to work on it or not.
> 
> 
> > You have two options:
> [..]
> > Or go ahead and create your own patch.
> 
> I will do this, but "only" on top of my subtitle filtering patchset
> because that's my current focus area and just two weeks ago I had to
> add a temporary hack for a related case which is about ASS dialog
> lines
> like:
> 
> Dialogue: 0,0:00:00.00,0:00:05.00,Default,,0,0,0,,{comment text:
> \}
> 
> Currently, ffmpeg does not recognize this as override code, even
> though
> it's valid in ASS that a backslash with the actual code doesn't appear
> immediately after the opening curly brace.
> What comes on top of this is that other subtitle decoders do NOT
> escape
> the curly braces like WebVTTdec and ass_bprint_text_event().
> When you look at SubRip_capability_tester.srt from the FATE suite, you
> can see that it contains ASS codes that are expected to be recognized
> and applied, but when there's normal text in curly braces without
> a backslash, it should be treated at normal text.
> 
> This is quite a mess that needs to be cleaned up with a plan and it's
> all related. Like I said already: A central point to this is the
> escaping
> and what's needed is a solution that can cover all of those things.
> 
> I had put this subject aside as I've been unsure about how to do it,
> but this discussion has been very helpful to see the issues more
> clearly.
> 
> How about separating the BIDI part from your patch? I'd see only two
> things remaining:
> 
> - Go through all text subtitle encoders and think/research whether
>   those m

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes

2022-02-05 Thread Soft Works


> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Oneric
> Sent: Saturday, February 5, 2022 11:00 PM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix
> handling of backslashes
> 
> On Sat, Feb 05, 2022 at 02:08:48 +, Soft Works wrote:
> > Let's try to approach this from a different side. Which case is
> > your [1/2] commit actually supposed to fix?
> 
> Escape backslashes when converting from WebVTT to not accidentally
> introduce active ASS sequences and replace the wrong backslash-escape
> in ff_ass_bprint_text_event with an escape that actually works.
> 
> > How did you test your patch?
> > Can we please go over an example?
> 
> Take a look at the attached WebVTT file.

Thanks a lot for the test file!


> We expect the second event to be rendered like this,
> as from WebVTT’s point of view it’s just normal text:
> 
>   our final \h approach \N into \ Coruscant.
> 
> What we currently get after conversion to ASS is like this though
> (pay attention to the number of spaces):
> 
>   our final   approach
>   into \ Coruscant.

Yup, no doubt that this is wrong. 


> If instead the word-joiner is appended as in my patch, the
> visual output matches the expectation (mail does not contain U+2060):
> 
>   our final \h approach \N into \ Coruscant.

I can also confirm that your patch would "work" with regards
to ass output when trying with both "old" libass and new libass.
I haven't tried with other implementations yet, but when this 
would turn out to be working with all usual implementations,
I might even be OK with the word-joiner.

But this is where the agreement ends. 

- If at all, the word-joiner insertion would need to be 
  limited to ASS output ONLY
- it would need to be controllable through an option in the ASS 
  encoder
- The word joiners should not be used in internal processing and 
  only be (optionally) added when encoding to ass
- Unfortunately, the FATE tests for the other subtitle formats
  do not include these sequences in the test source files, and
  that means, before making such change that might potentially 
  affect all other text subtitle encoders, those sequences would
  need to be added to make sure these conversion won't be affected
- Generally, those changes (also the BIDI mark insertion) should 
  be evaluated with regards to all text subtitle encoders,
  making sure there's no side effect.

You said:

> I’m not interested in reworking ffmpeg’s internal subtitle handling.
> The proposed patch is a clear improvement over the status quo which
> is plain incorrect. Within reasonable effort and sound arguments for
> it adjustments to the patch can be made; reworking ffmpeg internals is
> imo not “reasonable” effort to correct an uncontestedly wrong escape.

And that is a problem. The changes you are proposing are making
changes to ffmpeg’s internal subtitle handling, so you need to decide 
whether you want to work on it or not.


> You have two options:
[..]
> Or go ahead and create your own patch.

I will do this, but "only" on top of my subtitle filtering patchset
because that's my current focus area and just two weeks ago I had to
add a temporary hack for a related case which is about ASS dialog lines
like:

Dialogue: 0,0:00:00.00,0:00:05.00,Default,,0,0,0,,{comment text: \}

Currently, ffmpeg does not recognize this as override code, even though
it's valid in ASS that a backslash with the actual code doesn't appear
immediately after the opening curly brace.
What comes on top of this is that other subtitle decoders do NOT escape
the curly braces like WebVTTdec and ass_bprint_text_event().
When you look at SubRip_capability_tester.srt from the FATE suite, you
can see that it contains ASS codes that are expected to be recognized
and applied, but when there's normal text in curly braces without 
a backslash, it should be treated at normal text.

This is quite a mess that needs to be cleaned up with a plan and it's
all related. Like I said already: A central point to this is the escaping
and what's needed is a solution that can cover all of those things.

I had put this subject aside as I've been unsure about how to do it,
but this discussion has been very helpful to see the issues more clearly.

How about separating the BIDI part from your patch? I'd see only two
things remaining:

- Go through all text subtitle encoders and think/research whether 
  those marks would be acceptable in those formats or whether they
  would need to be removed (like now)
- Think about whether the Unicode bidi marks should be replaced back
  to the html-style codes
  It's not these wouldn't work, but it's again about visibility and 
  I tend to think that it would be preferable to have them visible 
  in the output

softworkz







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

To unsubscrib

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/{ass, webvttdec}: fix handling of backslashes

2022-02-05 Thread Oneric
On Sat, Feb 05, 2022 at 02:08:48 +, Soft Works wrote:
> Let's try to approach this from a different side. Which case is
> your [1/2] commit actually supposed to fix?

Escape backslashes when converting from WebVTT to not accidentally
introduce active ASS sequences and replace the wrong backslash-escape
in ff_ass_bprint_text_event with an escape that actually works.

> How did you test your patch?
> Can we please go over an example?

Take a look at the attached WebVTT file.
We expect the second event to be rendered like this,
as from WebVTT’s point of view it’s just normal text:

  our final \h approach \N into \ Coruscant.

What we currently get after conversion to ASS is like this though
(pay attention to the number of spaces):

  our final   approach
  into \ Coruscant.

If we escape \ as \\ like ff_ass_bprint_text_event currently does,
we instead get the following rendering which is also wrong:

  our final \  approach \
  into \\ Coruscant.

If instead the word-joiner is appended as in my patch, the
visual output matches the expectation (mail does not contain U+2060):

  our final \h approach \N into \ Coruscant.


You can confirm this for yourself by feeding the original WebVTT
to a native WebVTT renderer, e.g. https://zcorpan.github.io/live-webvtt-viewer/
and using ./ffmpeg -i test2.vtt -f ass tmp_conv.ass on
master, master + my patch and master + a modified patch to use \\
then watching how the converted file gets rendered by mpv, VLC or so.

I was not able to create a sample using ff_ass_bprint_text_event
as the only users seem to be teletext and some rawtext(?) subtitles
and I don't know how to create such files.
However, as nothing special happened to the \\ for WebVTT after this
sequence was inserted for the internal representation, and since
the preceding comment (wrongly) declares \\ to be an ASS rather than an
internal escape, it seems highly unlikely to behave any different.
You are very welcome to provide a sample using the
ff_ass_bprint_text_event codepath and keep_ass_markup=false
to verify or debunk this.
WEBVTT

00:00.000 --> 00:01.000
Senator, we're {A} making
 

00:01.000 --> 01:01.000
our final \h approach \N into \ Coruscant. 
___
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/2] avformat/matroskadec: Check desc_bytes

2022-02-05 Thread Andreas Rheinhardt
Andreas Rheinhardt:
> Michael Niedermayer:
>> Fixes: Division by 0
>> Fixes: 
>> 44035/clusterfuzz-testcase-minimized-ffmpeg_dem_WEBM_DASH_MANIFEST_fuzzer-4826721386364928
>>
>> Found-by: continuous fuzzing process 
>> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
>> Signed-off-by: Michael Niedermayer 
>> ---
>>  libavformat/matroskadec.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
>> index 5a9acfb247..f433391a16 100644
>> --- a/libavformat/matroskadec.c
>> +++ b/libavformat/matroskadec.c
>> @@ -4216,6 +4216,8 @@ static int64_t 
>> webm_dash_manifest_compute_bandwidth(AVFormatContext *s, int64_t
>>  do {
>>  int64_t desc_bytes = desc_end.end_offset - 
>> desc_beg.start_offset;
>>  int64_t desc_ns = desc_end.end_time_ns - 
>> desc_beg.start_time_ns;
>> +if (desc_bytes <= 0)
>> +return -1;
>>  double desc_sec = desc_ns / nano_seconds_per_second;
>>  double calc_bits_per_second = (desc_bytes * 8) / desc_sec;
>>  
> 
> This should give a declaration-after-statement error.
> 

s/error/warning/

- 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 1/2] avformat/matroskadec: Check desc_bytes

2022-02-05 Thread Andreas Rheinhardt
Michael Niedermayer:
> Fixes: Division by 0
> Fixes: 
> 44035/clusterfuzz-testcase-minimized-ffmpeg_dem_WEBM_DASH_MANIFEST_fuzzer-4826721386364928
> 
> Found-by: continuous fuzzing process 
> https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> Signed-off-by: Michael Niedermayer 
> ---
>  libavformat/matroskadec.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 5a9acfb247..f433391a16 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -4216,6 +4216,8 @@ static int64_t 
> webm_dash_manifest_compute_bandwidth(AVFormatContext *s, int64_t
>  do {
>  int64_t desc_bytes = desc_end.end_offset - 
> desc_beg.start_offset;
>  int64_t desc_ns = desc_end.end_time_ns - 
> desc_beg.start_time_ns;
> +if (desc_bytes <= 0)
> +return -1;
>  double desc_sec = desc_ns / nano_seconds_per_second;
>  double calc_bits_per_second = (desc_bytes * 8) / desc_sec;
>  

This should give a declaration-after-statement error.

- 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] avformat/mxfdec: add avlanguage dependency

2022-02-05 Thread Andreas Rheinhardt
Zane van Iperen:
> Signed-off-by: Zane van Iperen 
> ---
>  libavformat/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index 3dc6a479cc..6566e40cac 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -374,7 +374,7 @@ OBJS-$(CONFIG_MTV_DEMUXER)   += mtv.o
>  OBJS-$(CONFIG_MUSX_DEMUXER)  += musx.o
>  OBJS-$(CONFIG_MV_DEMUXER)+= mvdec.o
>  OBJS-$(CONFIG_MVI_DEMUXER)   += mvi.o
> -OBJS-$(CONFIG_MXF_DEMUXER)   += mxfdec.o mxf.o
> +OBJS-$(CONFIG_MXF_DEMUXER)   += mxfdec.o mxf.o avlanguage.o
>  OBJS-$(CONFIG_MXF_MUXER) += mxfenc.o mxf.o avc.o
>  OBJS-$(CONFIG_MXG_DEMUXER)   += mxg.o
>  OBJS-$(CONFIG_NC_DEMUXER)+= ncdec.o

LGTM.
(Seems like this landed (in 47c4df2203f) right after I checked all the
(de)muxers and codecs for missing dependencies.)

- 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 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

2022-02-05 Thread Chad Fraleigh
Since any [valid] value over 255 is impossible in the IPv4 protocol (the TTL 
field is only 8-bits), it should always be capped at 255 (for consistency) or 
return an invalid value error (the one I would suggest).

Despite VLC's reversed comment, using an int seems to be the exception to the 
rule (i.e. only linux and windows seem to use it [as-documented]), perhaps 
doing the unsigned char first and using the int as the fallback would be 
better? It's not really just a BSD thing, unless you also count LWIP and 
Solaris as BSD. Unless VLC's code history shows them doing it this way at one 
time and swapping it (but forgetting the comment) to fix a known bug?


On 2/4/2022 9:28 PM, lance.lmw...@gmail.com wrote:
> From: Limin Wang 
> 
> Suggested by zhilizhao, vlc project has solved the compatibility by
> the same way, so I borrowed the comments from vlc project.
> 
> Fix #ticket9449
> 
> Signed-off-by: Limin Wang 
> ---
>  libavformat/udp.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/udp.c b/libavformat/udp.c
> index 3dc79eb..34488d6 100644
> --- a/libavformat/udp.c
> +++ b/libavformat/udp.c
> @@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int 
> mcastTTL,
>  {
>  int protocol, cmd;
>  
> +/* There is some confusion in the world whether IP_MULTICAST_TTL
> + * takes a byte or an int as an argument.
> + * BSD seems to indicate byte so we are going with that and use
> + * int as a fallback to be safe */
>  switch (addr->sa_family) {
>  #ifdef IP_MULTICAST_TTL
>  case AF_INET:
> @@ -183,8 +187,15 @@ static int udp_set_multicast_ttl(int sockfd, int 
> mcastTTL,
>  }
>  
>  if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
> -ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> -return ff_neterrno();
> +/* BSD compatibility */
> +unsigned char ttl;
> +
> +ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt");
> +ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
> +if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
> +ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> +return ff_neterrno();
> +}
>  }
>  
>  return 0;
___
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/2] avformat/matroskadec: Check desc_bytes

2022-02-05 Thread Michael Niedermayer
Fixes: Division by 0
Fixes: 
44035/clusterfuzz-testcase-minimized-ffmpeg_dem_WEBM_DASH_MANIFEST_fuzzer-4826721386364928

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavformat/matroskadec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 5a9acfb247..f433391a16 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -4216,6 +4216,8 @@ static int64_t 
webm_dash_manifest_compute_bandwidth(AVFormatContext *s, int64_t
 do {
 int64_t desc_bytes = desc_end.end_offset - 
desc_beg.start_offset;
 int64_t desc_ns = desc_end.end_time_ns - 
desc_beg.start_time_ns;
+if (desc_bytes <= 0)
+return -1;
 double desc_sec = desc_ns / nano_seconds_per_second;
 double calc_bits_per_second = (desc_bytes * 8) / desc_sec;
 
-- 
2.17.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/2] avcodec/jpeglsdec: Increase range for N in ls_get_code_runterm() by using unsigned

2022-02-05 Thread Michael Niedermayer
Fixes: left shift of 32768 by 16 places cannot be represented in type 'int'
Fixes: Timeout
Fixes: 
44219/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SMVJPEG_fuzzer-4679455379947520
Fixes: 
44088/clusterfuzz-testcase-minimized-ffmpeg_AV_CODEC_ID_SMVJPEG_fuzzer-4885976600674304

Found-by: continuous fuzzing process 
https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
Signed-off-by: Michael Niedermayer 
---
 libavcodec/jpeglsdec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/jpeglsdec.c b/libavcodec/jpeglsdec.c
index 269c71dc18..6ca723f797 100644
--- a/libavcodec/jpeglsdec.c
+++ b/libavcodec/jpeglsdec.c
@@ -191,7 +191,7 @@ static inline int ls_get_code_runterm(GetBitContext *gb, 
JLSState *state,
 if (RItype)
 temp += state->N[Q] >> 1;
 
-for (k = 0; (state->N[Q] << k) < temp; k++)
+for (k = 0; ((unsigned)state->N[Q] << k) < temp; k++)
 ;
 
 #ifdef JLS_BROKEN
-- 
2.17.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] Demuxing and decoding raw RTP stream

2022-02-05 Thread Kevin Wang
Hi, I'm implementing a pipeline where I receive inbound RTP packets in
memory but I'm having trouble figuring out how to set up libavformat to
handle/unwrap the RTP packets.

I have all relevant information about the underlying codec necessary but
since it's h264 I cannot simply strip the RTP header trivially. I create
the input context with goInputFunction writing one packet per invocation.

void *readbuf = av_malloc(1500);
AVIOContext *avioreadctx = avio_alloc_context(readbuf, 1500, 0,
transcoder, &goInputFunction, NULL, NULL);
AVFormatContext *inputctx = avformat_alloc_context();
inputctx->pb = avioreadctx;
inputctx->flags |= AVFMT_FLAG_CUSTOM_IO;

When I open it with avformat_open_input(&inputctx, NULL, NULL, NULL) it
repeatedly calls the read function but doesn't actually progress. I suspect
because the RTP stream itself does not have enough information to fully
describe the codec? If I leave this open out, then av_read_frame(inputctx,
input_packet) down the road segfaults, I'm guessing because the input
context is uninitialized.

So to my question, is it possible to set the codec details that the SDP
would typically set, but manually?

I'm looking for an example of how to manually configure the AVFormatContext to
consume RTP packets without an SDP and setting up a UDP port listener.

Thanks,
Kevin
___
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] avfilter/adelay: Add command support

2022-02-05 Thread Paul B Mahol
Have this been tested?

For example changing delays multiple times per invocation?
___
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] avfilter: add dialog enhance audio filter

2022-02-05 Thread Paul B Mahol
Signed-off-by: Paul B Mahol 
---
 libavfilter/Makefile   |   1 +
 libavfilter/af_dialogenhance.c | 420 +
 libavfilter/allfilters.c   |   1 +
 3 files changed, 422 insertions(+)
 create mode 100644 libavfilter/af_dialogenhance.c

diff --git a/libavfilter/Makefile b/libavfilter/Makefile
index 282967144b..c4dfb9a41b 100644
--- a/libavfilter/Makefile
+++ b/libavfilter/Makefile
@@ -124,6 +124,7 @@ OBJS-$(CONFIG_CROSSFEED_FILTER)  += 
af_crossfeed.o
 OBJS-$(CONFIG_CRYSTALIZER_FILTER)+= af_crystalizer.o
 OBJS-$(CONFIG_DCSHIFT_FILTER)+= af_dcshift.o
 OBJS-$(CONFIG_DEESSER_FILTER)+= af_deesser.o
+OBJS-$(CONFIG_DIALOGENHANCE_FILTER)  += af_dialogenhance.o
 OBJS-$(CONFIG_DRMETER_FILTER)+= af_drmeter.o
 OBJS-$(CONFIG_DYNAUDNORM_FILTER) += af_dynaudnorm.o
 OBJS-$(CONFIG_EARWAX_FILTER) += af_earwax.o
diff --git a/libavfilter/af_dialogenhance.c b/libavfilter/af_dialogenhance.c
new file mode 100644
index 00..a22594ed67
--- /dev/null
+++ b/libavfilter/af_dialogenhance.c
@@ -0,0 +1,420 @@
+/*
+ * Copyright (c) 2022 Paul B Mahol
+ *
+ * This file is part of FFmpeg.
+ *
+ * FFmpeg is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * FFmpeg is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with FFmpeg; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include "libavutil/channel_layout.h"
+#include "libavutil/opt.h"
+#include "libavutil/tx.h"
+#include "audio.h"
+#include "avfilter.h"
+#include "filters.h"
+#include "internal.h"
+#include "window_func.h"
+
+#include 
+
+typedef struct AudioDialogEnhanceContext {
+const AVClass *class;
+
+double original, enhance;
+
+int fft_size;
+int overlap;
+int channels;
+
+float *window;
+float prev_vad;
+
+AVFrame *in;
+AVFrame *in_frame;
+AVFrame *out_dist_frame;
+AVFrame *windowed_frame;
+AVFrame *windowed_out;
+AVFrame *windowed_prev;
+AVFrame *center_frame;
+
+AVTXContext **tx_ctx, **itx_ctx;
+av_tx_fn tx_fn, itx_fn;
+} AudioDialogEnhanceContext;
+
+#define OFFSET(x) offsetof(AudioDialogEnhanceContext, x)
+#define FLAGS AV_OPT_FLAG_AUDIO_PARAM | AV_OPT_FLAG_FILTERING_PARAM | 
AV_OPT_FLAG_RUNTIME_PARAM
+
+static const AVOption dialogenhance_options[] = {
+{ "original", "set original center ratio", OFFSET(original), 
AV_OPT_TYPE_DOUBLE, {.dbl=1}, 0, 1, FLAGS },
+{ "enhance",  "set dialog enhance ratio",  OFFSET(enhance),  
AV_OPT_TYPE_DOUBLE, {.dbl=1}, 0, 1, FLAGS },
+{NULL}
+};
+
+AVFILTER_DEFINE_CLASS(dialogenhance);
+
+static int query_formats(AVFilterContext *ctx)
+{
+AVFilterFormats *formats = NULL;
+AVFilterChannelLayouts *layout = NULL;
+int ret;
+
+if ((ret = ff_add_format (&formats, AV_SAMPLE_FMT_FLTP )) 
< 0 ||
+(ret = ff_set_common_formats (ctx , formats)) 
< 0 ||
+(ret = ff_add_channel_layout (&layout , AV_CH_LAYOUT_STEREO)) 
< 0 ||
+(ret = ff_set_common_channel_layouts (ctx , layout )) 
< 0)
+return ret;
+
+return ff_set_common_all_samplerates(ctx);
+}
+
+static int config_input(AVFilterLink *inlink)
+{
+AVFilterContext *ctx = inlink->dst;
+AudioDialogEnhanceContext *s = ctx->priv;
+float scale = 1.f, iscale, overlap;
+int ret;
+
+s->fft_size = inlink->sample_rate > 10 ? 8192 : inlink->sample_rate > 
5 ? 4096 : 2048;
+s->overlap = s->fft_size / 4;
+
+s->window = av_calloc(s->fft_size, sizeof(*s->window));
+if (!s->window)
+return AVERROR(ENOMEM);
+
+s->in_frame   = ff_get_audio_buffer(inlink, s->fft_size * 4);
+s->center_frame   = ff_get_audio_buffer(inlink, s->fft_size * 4);
+s->out_dist_frame = ff_get_audio_buffer(inlink, s->fft_size * 4);
+s->windowed_frame = ff_get_audio_buffer(inlink, s->fft_size * 4);
+s->windowed_out   = ff_get_audio_buffer(inlink, s->fft_size * 4);
+s->windowed_prev  = ff_get_audio_buffer(inlink, s->fft_size * 4);
+if (!s->in_frame || !s->windowed_out || !s->windowed_prev ||
+!s->out_dist_frame || !s->windowed_frame || !s->center_frame)
+return AVERROR(ENOMEM);
+
+generate_window_func(s->window, s->fft_size, WFUNC_SINE, &overlap);
+
+s->channels = inlink->channels;
+
+s->tx_ctx = av_calloc(s->channels, sizeof(*s->tx_ctx));
+s->itx_ctx = av_calloc

Re: [FFmpeg-devel] [PATCH] mpegvideo_parser: check picture_structure for field order

2022-02-05 Thread James Almer

On 2/5/2022 9:51 AM, Tom Yan wrote:

the top_field_first bit is only used to indicate the field order
when the picture is a frame picture (which consists of two fields)
but not when it is a field picture (which consists of one single
top or bottom field).

Signed-off-by: Tom Yan 
---
  libavcodec/mpegvideo_parser.c | 7 +--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
index c5dc867d24..2ddcdb0f37 100644
--- a/libavcodec/mpegvideo_parser.c
+++ b/libavcodec/mpegvideo_parser.c
@@ -108,7 +108,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext 
*s,
  uint32_t start_code;
  int frame_rate_index, ext_type, bytes_left;
  int frame_rate_ext_n, frame_rate_ext_d;
-int top_field_first, repeat_first_field, progressive_frame;
+int picture_structure, top_field_first, repeat_first_field, 
progressive_frame;
  int horiz_size_ext, vert_size_ext, bit_rate_ext;
  int did_set_size=0;
  int set_dim_ret = 0;
@@ -181,6 +181,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext 
*s,
  break;
  case 0x8: /* picture coding extension */
  if (bytes_left >= 5) {
+picture_structure = buf[2] & 0x3;


You could maybe also set s->picture_structure with this.


  top_field_first = buf[3] & (1 << 7);
  repeat_first_field = buf[3] & (1 << 1);
  progressive_frame = buf[4] & (1 << 7);
@@ -199,7 +200,9 @@ static void mpegvideo_extract_headers(AVCodecParserContext 
*s,
  }
  
  if (!pc->progressive_sequence && !progressive_frame) {

-if (top_field_first)
+/* top_field_first is mandated to be 0 when
+   picture_structure is not 3 (i.e. not a frame 
picture) */
+if (top_field_first || picture_structure == 1)
  s->field_order = AV_FIELD_TT;
  else
  s->field_order = AV_FIELD_BB;


___
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] avformat/mpegts: add option max_packet_size

2022-02-05 Thread Gyan Doshi



On 2022-02-02 10:54 am, Gyan Doshi wrote:



On 2022-02-02 12:42 am, James Almer wrote:



On 2/1/2022 3:47 PM, Michael Niedermayer wrote:

On Sun, Jan 16, 2022 at 05:19:15AM +, Gyan Doshi wrote:
ffmpeg | branch: master | Gyan Doshi  | Wed Jan 
12 20:57:59 2022 +0530| [bca30570d28bbaa07badadabf55ec3589201a82f] 
| committer: Gyan Doshi


avformat/mpegts: add option max_packet_size

Makes maximum size of emitted packet user-tunable.

Default is existing 204800 bytes.

http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=bca30570d28bbaa07badadabf55ec3589201a82f 


---

  doc/demuxers.texi    |  4 
  libavformat/mpegts.c | 11 ++-
  2 files changed, 10 insertions(+), 5 deletions(-)


Since this commit there is occasional memory corruption occuring

   libavutil  57. 18.100 / 57. 18.100
   libavcodec 59. 20.100 / 59. 20.100
   libavformat    59. 17.101 / 59. 17.101
   libavdevice    59.  5.100 / 59.  5.100
   libavfilter 8. 25.100 /  8. 25.100
   libswscale  6.  5.100 /  6.  5.100
   libswresample   4.  4.100 /  4.  4.100
   libpostproc    56.  4.100 / 56.  4.100
invalid fastbin entry (free)
doom/rtp-video: line 14: 15621 Aborted (core dumped) 
./ffmpeg_g -bitexact -protocol_whitelist http,tcp,rtp,udp -i 
http://127.0.0.1:8080/test.sdp -bitexact -acodec mp2 -ab 64k -y -t 1 
-threads 1 $TMP/out.avi

Command exited with non-zero status 134


Is the demuxer used in this scenario mpegts or mpegtsraw? The new 
option was added to the former but not the latter, yet if both read 
MpegTSContext.max_packet_size, for mpegtsraw it will always be 0.


The option might need to be added to that demuxer too.


The init appears to be via  avpriv_mpegts_parse_open in mpegts.c

which has this note above,

    /* parsing functions - called from other demuxers such as RTP */

and in it, a few of the context fields are manually initialized. I 
suspect hardcoding a value here may be enough.


Looks to be enough. Sent patch "avformat/mpegts: initialize 
max_packet_size when sub-demuxer"  to address this.


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] [PATCH] avformat/mpegts: initialize max_packet_size when sub-demuxer

2022-02-05 Thread Gyan Doshi
bca30570d2 added a user option to set max_packet_size replacing
a hardcoded value. This had a side-effect of leaving the field
set to 0 when packet demuxing is carried out from another demuxer
using avpriv functions, which could lead to demux failure.

Hardcoded max_packet_size inside avpriv_mpegts_parse_open to
2048000 to avoid this. Value chosen to be 10x that of default value
to accommodate large payloads.
---
 libavformat/mpegts.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
index da15223b8a..e23f596490 100644
--- a/libavformat/mpegts.c
+++ b/libavformat/mpegts.c
@@ -3377,6 +3377,7 @@ MpegTSContext *avpriv_mpegts_parse_open(AVFormatContext 
*s)
 return NULL;
 /* no stream case, currently used by RTP */
 ts->raw_packet_size = TS_PACKET_SIZE;
+ts->max_packet_size = 2048000;
 ts->stream = s;
 ts->auto_guess = 1;
 
-- 
2.34.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] mpegvideo_parser: check picture_structure for field order

2022-02-05 Thread Tom Yan
It can probably be simplified to this:

...
if (progressive_frame)
s->field_order = AV_FIELD_PROGRESSIVE;
/* top_field_first is mandated to be 0 when
   picture_structure is not 3 (i.e. not a frame picture) */
else if (top_field_first || picture_structure == 1)
s->field_order = AV_FIELD_TT;
else
s->field_order = AV_FIELD_BB;
...

since progressive_sequence == 1 is illegal anyway if not all the
pictures/frames in the sequence are marked with progressive_frame ==
1.

If we consider commit 88e2dc7 proper, i.e. we should set
s->field_order to AV_FIELD_PROGRESSIVE when (processive_sequence == 0
and progressive_frame == 1), there's not really a point to check
processive_sequence at all.

On Sat, 5 Feb 2022 at 20:51, Tom Yan  wrote:
>
> the top_field_first bit is only used to indicate the field order
> when the picture is a frame picture (which consists of two fields)
> but not when it is a field picture (which consists of one single
> top or bottom field).
>
> Signed-off-by: Tom Yan 
> ---
>  libavcodec/mpegvideo_parser.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
> index c5dc867d24..2ddcdb0f37 100644
> --- a/libavcodec/mpegvideo_parser.c
> +++ b/libavcodec/mpegvideo_parser.c
> @@ -108,7 +108,7 @@ static void 
> mpegvideo_extract_headers(AVCodecParserContext *s,
>  uint32_t start_code;
>  int frame_rate_index, ext_type, bytes_left;
>  int frame_rate_ext_n, frame_rate_ext_d;
> -int top_field_first, repeat_first_field, progressive_frame;
> +int picture_structure, top_field_first, repeat_first_field, 
> progressive_frame;
>  int horiz_size_ext, vert_size_ext, bit_rate_ext;
>  int did_set_size=0;
>  int set_dim_ret = 0;
> @@ -181,6 +181,7 @@ static void 
> mpegvideo_extract_headers(AVCodecParserContext *s,
>  break;
>  case 0x8: /* picture coding extension */
>  if (bytes_left >= 5) {
> +picture_structure = buf[2] & 0x3;
>  top_field_first = buf[3] & (1 << 7);
>  repeat_first_field = buf[3] & (1 << 1);
>  progressive_frame = buf[4] & (1 << 7);
> @@ -199,7 +200,9 @@ static void 
> mpegvideo_extract_headers(AVCodecParserContext *s,
>  }
>
>  if (!pc->progressive_sequence && !progressive_frame) 
> {
> -if (top_field_first)
> +/* top_field_first is mandated to be 0 when
> +   picture_structure is not 3 (i.e. not a frame 
> picture) */
> +if (top_field_first || picture_structure == 1)
>  s->field_order = AV_FIELD_TT;
>  else
>  s->field_order = AV_FIELD_BB;
> --
> 2.35.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] avformat/mxfdec: add avlanguage dependency

2022-02-05 Thread Zane van Iperen
Signed-off-by: Zane van Iperen 
---
 libavformat/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavformat/Makefile b/libavformat/Makefile
index 3dc6a479cc..6566e40cac 100644
--- a/libavformat/Makefile
+++ b/libavformat/Makefile
@@ -374,7 +374,7 @@ OBJS-$(CONFIG_MTV_DEMUXER)   += mtv.o
 OBJS-$(CONFIG_MUSX_DEMUXER)  += musx.o
 OBJS-$(CONFIG_MV_DEMUXER)+= mvdec.o
 OBJS-$(CONFIG_MVI_DEMUXER)   += mvi.o
-OBJS-$(CONFIG_MXF_DEMUXER)   += mxfdec.o mxf.o
+OBJS-$(CONFIG_MXF_DEMUXER)   += mxfdec.o mxf.o avlanguage.o
 OBJS-$(CONFIG_MXF_MUXER) += mxfenc.o mxf.o avc.o
 OBJS-$(CONFIG_MXG_DEMUXER)   += mxg.o
 OBJS-$(CONFIG_NC_DEMUXER)+= ncdec.o
-- 
2.33.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] mpegvideo_parser: check picture_structure for field order

2022-02-05 Thread Tom Yan
the top_field_first bit is only used to indicate the field order
when the picture is a frame picture (which consists of two fields)
but not when it is a field picture (which consists of one single
top or bottom field).

Signed-off-by: Tom Yan 
---
 libavcodec/mpegvideo_parser.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavcodec/mpegvideo_parser.c b/libavcodec/mpegvideo_parser.c
index c5dc867d24..2ddcdb0f37 100644
--- a/libavcodec/mpegvideo_parser.c
+++ b/libavcodec/mpegvideo_parser.c
@@ -108,7 +108,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext 
*s,
 uint32_t start_code;
 int frame_rate_index, ext_type, bytes_left;
 int frame_rate_ext_n, frame_rate_ext_d;
-int top_field_first, repeat_first_field, progressive_frame;
+int picture_structure, top_field_first, repeat_first_field, 
progressive_frame;
 int horiz_size_ext, vert_size_ext, bit_rate_ext;
 int did_set_size=0;
 int set_dim_ret = 0;
@@ -181,6 +181,7 @@ static void mpegvideo_extract_headers(AVCodecParserContext 
*s,
 break;
 case 0x8: /* picture coding extension */
 if (bytes_left >= 5) {
+picture_structure = buf[2] & 0x3;
 top_field_first = buf[3] & (1 << 7);
 repeat_first_field = buf[3] & (1 << 1);
 progressive_frame = buf[4] & (1 << 7);
@@ -199,7 +200,9 @@ static void mpegvideo_extract_headers(AVCodecParserContext 
*s,
 }
 
 if (!pc->progressive_sequence && !progressive_frame) {
-if (top_field_first)
+/* top_field_first is mandated to be 0 when
+   picture_structure is not 3 (i.e. not a frame 
picture) */
+if (top_field_first || picture_structure == 1)
 s->field_order = AV_FIELD_TT;
 else
 s->field_order = AV_FIELD_BB;
-- 
2.35.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 v3 3/3] avformat/udp: remove IPPROTO_IPV6 macro

2022-02-05 Thread lance . lmwang
From: Limin Wang 

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

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 1871acf..9f6ab1b 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -175,7 +175,7 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
 cmd  = IP_MULTICAST_TTL;
 break;
 #endif
-#if defined(IPPROTO_IPV6) && defined(IPV6_MULTICAST_HOPS)
+#ifdef IPV6_MULTICAST_HOPS
 case AF_INET6:
 protocol = IPPROTO_IPV6;
 cmd  = IPV6_MULTICAST_HOPS;
-- 
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] [PATCH v3 2/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

2022-02-05 Thread lance . lmwang
From: Limin Wang 

Suggested by zhilizhao, vlc project has solved the compatibility by
the same way, so I borrowed the comments from vlc project.

Fix #ticket9449

Signed-off-by: Limin Wang 
---
 libavformat/udp.c | 15 +--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 8178d0e..1871acf 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
 {
 int protocol, cmd;
 
+/* There is some confusion in the world whether IP_MULTICAST_TTL
+ * takes a byte or an int as an argument.
+ * BSD seems to indicate byte so we are going with that and use
+ * int and fall back to byte to be safe */
 switch (addr->sa_family) {
 #ifdef IP_MULTICAST_TTL
 case AF_INET:
@@ -182,8 +186,15 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
 }
 
 if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
-ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV4/IPV6 MULTICAST 
TTL)");
-return ff_neterrno();
+/* BSD compatibility */
+unsigned char ttl;
+
+ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt(IPV4/IPV6 MULTICAST 
TTL)");
+ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
+if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
+ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV4/IPV6 
MULTICAST TTL)");
+return ff_neterrno();
+}
 }
 
 return 0;
-- 
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] [PATCH v3 1/3] avformat/udp: use one setsockopt for ipv4/ipv6

2022-02-05 Thread lance . lmwang
From: Limin Wang 

Signed-off-by: Limin Wang 
---
 libavformat/udp.c | 32 
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 83c042d..8178d0e 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -162,22 +162,30 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
  struct sockaddr *addr,
  void *logctx)
 {
+int protocol, cmd;
+
+switch (addr->sa_family) {
 #ifdef IP_MULTICAST_TTL
-if (addr->sa_family == AF_INET) {
-if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, 
sizeof(mcastTTL)) < 0) {
-ff_log_net_error(logctx, AV_LOG_ERROR, 
"setsockopt(IP_MULTICAST_TTL)");
-return ff_neterrno();
-}
-}
+case AF_INET:
+protocol = IPPROTO_IP;
+cmd  = IP_MULTICAST_TTL;
+break;
 #endif
 #if defined(IPPROTO_IPV6) && defined(IPV6_MULTICAST_HOPS)
-if (addr->sa_family == AF_INET6) {
-if (setsockopt(sockfd, IPPROTO_IPV6, IPV6_MULTICAST_HOPS, &mcastTTL, 
sizeof(mcastTTL)) < 0) {
-ff_log_net_error(logctx, AV_LOG_ERROR, 
"setsockopt(IPV6_MULTICAST_HOPS)");
-return ff_neterrno();
-}
-}
+case AF_INET6:
+protocol = IPPROTO_IPV6;
+cmd  = IPV6_MULTICAST_HOPS;
+break;
 #endif
+default:
+return 0;
+}
+
+if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
+ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt(IPV4/IPV6 MULTICAST 
TTL)");
+return ff_neterrno();
+}
+
 return 0;
 }
 
-- 
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".


Re: [FFmpeg-devel] [PATCH v2 1/3] avformat/udp: use one setsockopt for ipv4/ipv6

2022-02-05 Thread lance . lmwang
On Sat, Feb 05, 2022 at 10:58:08AM +0100, Marton Balint wrote:
> 
> 
> On Sat, 5 Feb 2022, lance.lmw...@gmail.com wrote:
> 
> > From: Limin Wang 
> > 
> > Signed-off-by: Limin Wang 
> > ---
> > libavformat/udp.c | 31 ---
> > 1 file changed, 20 insertions(+), 11 deletions(-)
> > 
> > diff --git a/libavformat/udp.c b/libavformat/udp.c
> > index 83c042d..3dc79eb 100644
> > --- a/libavformat/udp.c
> > +++ b/libavformat/udp.c
> > @@ -162,22 +162,31 @@ static int udp_set_multicast_ttl(int sockfd, int 
> > mcastTTL,
> >  struct sockaddr *addr,
> >  void *logctx)
> > {
> > +int protocol, cmd;
> > +
> > +switch (addr->sa_family) {
> > #ifdef IP_MULTICAST_TTL
> > -if (addr->sa_family == AF_INET) {
> > -if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, 
> > sizeof(mcastTTL)) < 0) {
> > -ff_log_net_error(logctx, AV_LOG_ERROR, 
> > "setsockopt(IP_MULTICAST_TTL)");
> > -return ff_neterrno();
> > -}
> > -}
> > +case AF_INET:
> > +protocol = IPPROTO_IP;
> > +cmd  = IP_MULTICAST_TTL;
> > +break;
> > #endif
> > #if defined(IPPROTO_IPV6) && defined(IPV6_MULTICAST_HOPS)
> > -if (addr->sa_family == AF_INET6) {
> > -if (setsockopt(sockfd, IPPROTO_IPV6, IPV6_MULTICAST_HOPS, 
> > &mcastTTL, sizeof(mcastTTL)) < 0) {
> > -ff_log_net_error(logctx, AV_LOG_ERROR, 
> > "setsockopt(IPV6_MULTICAST_HOPS)");
> > +case AF_INET6:
> > +protocol = IPPROTO_IPV6;
> > +cmd  = IPV6_MULTICAST_HOPS;
> > +break;
> > +#endif
> > +default:
> > +errno = EAFNOSUPPORT;
> 
> This is not portable, ff_neterrno is different for winsock. Maybe you should
> simply remove the default case, to make it work like the old code, and not
> mix behaviour changes with factorization.

OK, will remove the default case to same with old code.

> 
> > return ff_neterrno();
> > -}
> > }
> > -#endif
> > +
> > +if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 
> > 0) {
> > +ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> 
> "setsockopt(IPV4/IPV6 MULTICAST TTL)", so we know that the issue was with
> TTL setting?

Sure, it looks better. will update the patch.

> 
> 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".

-- 
Thanks,
Limin Wang
___
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/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

2022-02-05 Thread lance . lmwang
On Sat, Feb 05, 2022 at 10:59:55AM +0100, Marton Balint wrote:
> 
> 
> On Sat, 5 Feb 2022, lance.lmw...@gmail.com wrote:
> 
> > From: Limin Wang 
> > 
> > Suggested by zhilizhao, vlc project has solved the compatibility by
> > the same way, so I borrowed the comments from vlc project.
> > 
> > Fix #ticket9449
> > 
> > Signed-off-by: Limin Wang 
> > ---
> > libavformat/udp.c | 15 +--
> > 1 file changed, 13 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/udp.c b/libavformat/udp.c
> > index 3dc79eb..34488d6 100644
> > --- a/libavformat/udp.c
> > +++ b/libavformat/udp.c
> > @@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int 
> > mcastTTL,
> > {
> > int protocol, cmd;
> > 
> > +/* There is some confusion in the world whether IP_MULTICAST_TTL
> > + * takes a byte or an int as an argument.
> > + * BSD seems to indicate byte so we are going with that and use
> > + * int as a fallback to be safe */
> 
> The code does the opposite. Tries int and falls back to byte. Either change
> code or change comment.

Yes, good catch, so vlc use wrong comments. I'll change comments like below.
+ * BSD seems to indicate byte so we are going with that and tries
+ * int and falls back to byte to be safe */

> 
> Regards,
> Marton
> 
> > switch (addr->sa_family) {
> > #ifdef IP_MULTICAST_TTL
> > case AF_INET:
> > @@ -183,8 +187,15 @@ static int udp_set_multicast_ttl(int sockfd, int 
> > mcastTTL,
> > }
> > 
> > if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) 
> > {
> > -ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> > -return ff_neterrno();
> > +/* BSD compatibility */
> > +unsigned char ttl;
> > +
> > +ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt");
> > +ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
> > +if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
> > +ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
> > +return ff_neterrno();
> > +}
> > }
> > 
> > return 0;
> > -- 
> > 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".

-- 
Thanks,
Limin Wang
___
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/3] avformat/udp: Fix IP_MULTICAST_TTL for BSD compatibility

2022-02-05 Thread Marton Balint




On Sat, 5 Feb 2022, lance.lmw...@gmail.com wrote:


From: Limin Wang 

Suggested by zhilizhao, vlc project has solved the compatibility by
the same way, so I borrowed the comments from vlc project.

Fix #ticket9449

Signed-off-by: Limin Wang 
---
libavformat/udp.c | 15 +--
1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 3dc79eb..34488d6 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -164,6 +164,10 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
{
int protocol, cmd;

+/* There is some confusion in the world whether IP_MULTICAST_TTL
+ * takes a byte or an int as an argument.
+ * BSD seems to indicate byte so we are going with that and use
+ * int as a fallback to be safe */


The code does the opposite. Tries int and falls back to byte. Either 
change code or change comment.


Regards,
Marton


switch (addr->sa_family) {
#ifdef IP_MULTICAST_TTL
case AF_INET:
@@ -183,8 +187,15 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
}

if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
-ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
-return ff_neterrno();
+/* BSD compatibility */
+unsigned char ttl;
+
+ff_log_net_error(logctx, AV_LOG_DEBUG, "setsockopt");
+ttl = (unsigned char)(( mcastTTL > 255 ) ? 255 : mcastTTL);
+if (setsockopt(sockfd, protocol, cmd, &ttl, sizeof(ttl)) < 0) {
+ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");
+return ff_neterrno();
+}
}

return 0;
--
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/3] avformat/udp: use one setsockopt for ipv4/ipv6

2022-02-05 Thread Marton Balint




On Sat, 5 Feb 2022, lance.lmw...@gmail.com wrote:


From: Limin Wang 

Signed-off-by: Limin Wang 
---
libavformat/udp.c | 31 ---
1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/libavformat/udp.c b/libavformat/udp.c
index 83c042d..3dc79eb 100644
--- a/libavformat/udp.c
+++ b/libavformat/udp.c
@@ -162,22 +162,31 @@ static int udp_set_multicast_ttl(int sockfd, int mcastTTL,
 struct sockaddr *addr,
 void *logctx)
{
+int protocol, cmd;
+
+switch (addr->sa_family) {
#ifdef IP_MULTICAST_TTL
-if (addr->sa_family == AF_INET) {
-if (setsockopt(sockfd, IPPROTO_IP, IP_MULTICAST_TTL, &mcastTTL, 
sizeof(mcastTTL)) < 0) {
-ff_log_net_error(logctx, AV_LOG_ERROR, 
"setsockopt(IP_MULTICAST_TTL)");
-return ff_neterrno();
-}
-}
+case AF_INET:
+protocol = IPPROTO_IP;
+cmd  = IP_MULTICAST_TTL;
+break;
#endif
#if defined(IPPROTO_IPV6) && defined(IPV6_MULTICAST_HOPS)
-if (addr->sa_family == AF_INET6) {
-if (setsockopt(sockfd, IPPROTO_IPV6, IPV6_MULTICAST_HOPS, &mcastTTL, 
sizeof(mcastTTL)) < 0) {
-ff_log_net_error(logctx, AV_LOG_ERROR, 
"setsockopt(IPV6_MULTICAST_HOPS)");
+case AF_INET6:
+protocol = IPPROTO_IPV6;
+cmd  = IPV6_MULTICAST_HOPS;
+break;
+#endif
+default:
+errno = EAFNOSUPPORT;


This is not portable, ff_neterrno is different for winsock. Maybe you 
should simply remove the default case, to make it work like the old code, 
and not mix behaviour changes with factorization.



return ff_neterrno();
-}
}
-#endif
+
+if (setsockopt(sockfd, protocol, cmd, &mcastTTL, sizeof(mcastTTL)) < 0) {
+ff_log_net_error(logctx, AV_LOG_ERROR, "setsockopt");


"setsockopt(IPV4/IPV6 MULTICAST TTL)", so we know that the issue was with 
TTL setting?


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 08/13] avpriv_find_start_code(): add parameter for ignoring history

2022-02-05 Thread Scott Theisen

On 2/5/22 01:10, Andreas Rheinhardt wrote:

1. This patch is absolutely unacceptable: It breaks ABI.
2. I am surprised that there is apparently only one user that actually
wants this feature of state being an input parameter, namely
(ff_)mpeg1_find_frame_end(). This means that this loop done before the
new check should actually be made by this caller alone, obviating the
need to change the signature.
3. Given that no user of this in libavformat wants this feature,
changing it is IMO acceptable from an ABI-point of view.


I'll look into it.


4. There might be some slight changes introduced by this though:
Consider 00 00 01 00 00 01 xy. If the initial state is -1, a call to
avpriv_find_start_code() will treat the initial four bytes as start code
and return a pointer to the byte preceding the second 0x01. If the user
does not reset the start code between calls to avpriv_find_start_code(),
the second call will combine the last zero byte of the start code with
the rest to form another start code that partially overlaps with the
earlier one. This is (probably) invalid data (definitely for H.262,
H.264 and HEVC).


With input buffer 00 00 01 00 00 01 xy ...
The code in master will (incorrectly) find a second start code at offset 7.
It would need to check if (*start_code == 0x100) and invalidate it.

-Scott

___
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/7] avcodec/cbs_mpeg2: Simplify splitting fragment

2022-02-05 Thread Scott Theisen

On 2/4/22 14:01, Scott Theisen wrote:


I assume this is inspired by my patch series.  Did you see v2 of my 
patch series where I made similar and more extensive clarifications?


The check for sequence_end_codes should not be in the loop.  It harms 
readability, although you also eliminated the branch.


I retract my comments.

-Scott
___
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 6/7] avcodec/internal.h: Move avpriv_find_start_code() to startcode.h

2022-02-05 Thread Andreas Rheinhardt
Scott Theisen:
> On 2/4/22 10:16, Andreas Rheinhardt wrote:
>> This is by definition the appropriate place for it.
>> Remove all the now unnecessary libavcodec/internal.h inclusions;
>> also remove other unnecessary headers from the affected files.
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
> 
>>
>> diff --git a/libavcodec/utils.c b/libavcodec/utils.c
>> index b19befef21..c7c7323351 100644
>> --- a/libavcodec/utils.c
>> +++ b/libavcodec/utils.c
>> @@ -27,7 +27,6 @@
>>     #include "config.h"
>>   #include "libavutil/avassert.h"
>> -#include "libavutil/avstring.h"
>>   #include "libavutil/channel_layout.h"
>>   #include "libavutil/intreadwrite.h"
>>   #include "libavutil/mem.h"
>> @@ -40,12 +39,9 @@
>>   #include "thread.h"
>>   #include "internal.h"
>>   #include "put_bits.h"
>> -#include "raw.h"
>> +#include "startcode.h"
>>   #include 
>> -#include 
>> -#include 
>>   #include 
>> -#include 
>>     void av_fast_padded_malloc(void *ptr, unsigned int *size, size_t
>> min_size)
>>   {
> 
> Shouldn't you also move the definition to startcode.c as well? Then
> utils.c doesn't need startcode.h.

startcode.c is only compiled conditionally and the check is done in
configure, so I would have to add new configure dependencies. Yet
configure dependencies only take effect after a reconfigure, but there
is no good way to force a reconfigure (only adding a new codec, filter
or format forces this).
(There is a workaround: Modify the files containing the codecs, filters
or formats lists somehow, but this is not nice.)

- 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".