Re: [FFmpeg-devel] [PATCH 1/2] avcodec/movtextenc: Check for too many styles

[Guo, Yejun]

> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Andreas Rheinhardt
> Sent: 2021年2月21日 12:39
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/movtextenc: Check for too
> many styles
> 
> Guo, Yejun:
> >
> >
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
> >> Andreas Rheinhardt
> >> Sent: 2021年2月21日 12:12
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/movtextenc: Check for
> >> too many styles
> >>
> >> Guo, Yejun:
> >>>
> >>>
>  -Original Message-
>  From: ffmpeg-devel  On Behalf Of
>  Andreas Rheinhardt
>  Sent: 2021年2月21日 9:41
>  To: ffmpeg-devel@ffmpeg.org
>  Cc: Andreas Rheinhardt 
>  Subject: [FFmpeg-devel] [PATCH 1/2] avcodec/movtextenc: Check for
>  too many styles
> 
>  The counter for the number of styles is written on two bytes, ergo
>  anything > UINT16_MAX is invalid. This also fixes a compiler
>  warning because of a tautologically true check on 64bit systems.
> 
>  Signed-off-by: Andreas Rheinhardt 
>  ---
>  A better solution would be to error out as soon as the byte length
>  of a subtitle exceeds UINT16_MAX; yet for this one would have to
>  modify all of ass_split to allow the callbacks to return errors.
> 
>   libavcodec/movtextenc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
>  diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c
>  index 1bef21e0b9..cf30adbd0a 100644
>  --- a/libavcodec/movtextenc.c
>  +++ b/libavcodec/movtextenc.c
>  @@ -355,7 +355,7 @@ static int mov_text_style_start(MovTextContext
> *s)
>   StyleBox *tmp;
> 
>   // last style != defaults, end the style entry and start a new
> one
>  -if (s->count + 1 > SIZE_MAX / sizeof(*s->style_attributes) ||
>  +if (s->count + 1 > FFMIN(SIZE_MAX /
>  + sizeof(*s->style_attributes), UINT16_MAX) ||
> >>>
> >>> hi, logically, I think the result of FFMIN(SIZE_MAX /
> >> sizeof(*s->style_attributes), UINT16_MAX) is always UINT16_MAX, we
> >> may just use 's->count + 1 > UINT16_MAX'.
> >>>
> >>
> >> And why?
> >
> > For the original code below, the compiler reports that it is always false.
> > s->count + 1 > SIZE_MAX / sizeof(*s->style_attributes)
> >
> > Since s->count is unsigned int, imho, it means that SIZE_MAX /
> > sizeof(*s->style_attributes) is always larger than uint_max, and so of 
> > course
> larger than uint16_max.
> >
> > another is that:
> > sizeof(*s->style_attributes) is 12,
> > SIZE_MAX: 18446744073709551615
> > UINT16_MAX: 65535
> >
> > checked with code:
> > #include 
> > #include 
> > int main()
> > {
> > printf("SIZE_MAX: %lu\nUINT16_MAX: %d\n", SIZE_MAX,
> UINT16_MAX); }
> >
> FYI: The C standard does not mandate the sizes for most types (the uintx_t
> types are an exception). It also gives the compiler absolute freedom on how
> much padding to add to a structure. A compiler could very well add megabytes
> of padding to StyleBox. The numbers you provide only pertain to one (or
> actually none, see below) implementation, not to all of them. It is like
> someone claiming to prove a general theorem by only checking it for one
> example.
> 
> Several of your statements are btw not only false for a hypothetical system
> compliant with the C standard. They are false on common systems:
> "SIZE_MAX / sizeof(*s->style_attributes) is always larger than uint_max"
> is wrong on ordinary 32bit systems (where SIZE_MAX and UINT_MAX typically
> coincide). And "sizeof(*s->style_attributes) is 12". Due to padding it is 
> normally
> 16; it could be reduced to 12 (for ordinary
> systems) by rearranging its elements.
> 

thanks for the lucid explanation, yes, you're right. 
(and first to know that megabytes of padding is possible)

btw, just as a learning, is there possible a real system where uint_max < 
uint32_max?
If no, we might say that: size_max >= uint_max >= uint32_max > uint16_max.


___
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/movtextenc: Check for too many styles

[Andreas Rheinhardt]

Guo, Yejun:
> 
> 
>> -Original Message-
>> From: ffmpeg-devel  On Behalf Of
>> Andreas Rheinhardt
>> Sent: 2021年2月21日 12:12
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/movtextenc: Check for too
>> many styles
>>
>> Guo, Yejun:
>>>
>>>
 -Original Message-
 From: ffmpeg-devel  On Behalf Of
 Andreas Rheinhardt
 Sent: 2021年2月21日 9:41
 To: ffmpeg-devel@ffmpeg.org
 Cc: Andreas Rheinhardt 
 Subject: [FFmpeg-devel] [PATCH 1/2] avcodec/movtextenc: Check for too
 many styles

 The counter for the number of styles is written on two bytes, ergo
 anything > UINT16_MAX is invalid. This also fixes a compiler warning
 because of a tautologically true check on 64bit systems.

 Signed-off-by: Andreas Rheinhardt 
 ---
 A better solution would be to error out as soon as the byte length of
 a subtitle exceeds UINT16_MAX; yet for this one would have to modify
 all of ass_split to allow the callbacks to return errors.

  libavcodec/movtextenc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

 diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c index
 1bef21e0b9..cf30adbd0a 100644
 --- a/libavcodec/movtextenc.c
 +++ b/libavcodec/movtextenc.c
 @@ -355,7 +355,7 @@ static int mov_text_style_start(MovTextContext *s)
  StyleBox *tmp;

  // last style != defaults, end the style entry and start a new one
 -if (s->count + 1 > SIZE_MAX / sizeof(*s->style_attributes) ||
 +if (s->count + 1 > FFMIN(SIZE_MAX /
 + sizeof(*s->style_attributes), UINT16_MAX) ||
>>>
>>> hi, logically, I think the result of FFMIN(SIZE_MAX /
>> sizeof(*s->style_attributes), UINT16_MAX) is always UINT16_MAX, we may just
>> use 's->count + 1 > UINT16_MAX'.
>>>
>>
>> And why?
> 
> For the original code below, the compiler reports that it is always false.
> s->count + 1 > SIZE_MAX / sizeof(*s->style_attributes)
> 
> Since s->count is unsigned int, imho, it means that SIZE_MAX / 
> sizeof(*s->style_attributes)
> is always larger than uint_max, and so of course larger than uint16_max.
> 
> another is that:
> sizeof(*s->style_attributes) is 12,
> SIZE_MAX: 18446744073709551615
> UINT16_MAX: 65535
> 
> checked with code:
> #include 
> #include 
> int main()
> {
> printf("SIZE_MAX: %lu\nUINT16_MAX: %d\n", SIZE_MAX, UINT16_MAX);
> }
> 
FYI: The C standard does not mandate the sizes for most types (the
uintx_t types are an exception). It also gives the compiler absolute
freedom on how much padding to add to a structure. A compiler could very
well add megabytes of padding to StyleBox. The numbers you provide only
pertain to one (or actually none, see below) implementation, not to all
of them. It is like someone claiming to prove a general theorem by only
checking it for one example.

Several of your statements are btw not only false for a hypothetical
system compliant with the C standard. They are false on common systems:
"SIZE_MAX / sizeof(*s->style_attributes) is always larger than uint_max"
is wrong on ordinary 32bit systems (where SIZE_MAX and UINT_MAX
typically coincide). And "sizeof(*s->style_attributes) is 12". Due to
padding it is normally 16; it could be reduced to 12 (for ordinary
systems) by rearranging its elements.

- 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] avcodec/movtextenc: Check for too many styles

[Guo, Yejun]

> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Andreas Rheinhardt
> Sent: 2021年2月21日 12:12
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 1/2] avcodec/movtextenc: Check for too
> many styles
> 
> Guo, Yejun:
> >
> >
> >> -Original Message-
> >> From: ffmpeg-devel  On Behalf Of
> >> Andreas Rheinhardt
> >> Sent: 2021年2月21日 9:41
> >> To: ffmpeg-devel@ffmpeg.org
> >> Cc: Andreas Rheinhardt 
> >> Subject: [FFmpeg-devel] [PATCH 1/2] avcodec/movtextenc: Check for too
> >> many styles
> >>
> >> The counter for the number of styles is written on two bytes, ergo
> >> anything > UINT16_MAX is invalid. This also fixes a compiler warning
> >> because of a tautologically true check on 64bit systems.
> >>
> >> Signed-off-by: Andreas Rheinhardt 
> >> ---
> >> A better solution would be to error out as soon as the byte length of
> >> a subtitle exceeds UINT16_MAX; yet for this one would have to modify
> >> all of ass_split to allow the callbacks to return errors.
> >>
> >>  libavcodec/movtextenc.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c index
> >> 1bef21e0b9..cf30adbd0a 100644
> >> --- a/libavcodec/movtextenc.c
> >> +++ b/libavcodec/movtextenc.c
> >> @@ -355,7 +355,7 @@ static int mov_text_style_start(MovTextContext *s)
> >>  StyleBox *tmp;
> >>
> >>  // last style != defaults, end the style entry and start a new one
> >> -if (s->count + 1 > SIZE_MAX / sizeof(*s->style_attributes) ||
> >> +if (s->count + 1 > FFMIN(SIZE_MAX /
> >> + sizeof(*s->style_attributes), UINT16_MAX) ||
> >
> > hi, logically, I think the result of FFMIN(SIZE_MAX /
> sizeof(*s->style_attributes), UINT16_MAX) is always UINT16_MAX, we may just
> use 's->count + 1 > UINT16_MAX'.
> >
> 
> And why?

For the original code below, the compiler reports that it is always false.
s->count + 1 > SIZE_MAX / sizeof(*s->style_attributes)

Since s->count is unsigned int, imho, it means that SIZE_MAX / 
sizeof(*s->style_attributes)
is always larger than uint_max, and so of course larger than uint16_max.

another is that:
sizeof(*s->style_attributes) is 12,
SIZE_MAX: 18446744073709551615
UINT16_MAX: 65535

checked with code:
#include 
#include 
int main()
{
printf("SIZE_MAX: %lu\nUINT16_MAX: %d\n", SIZE_MAX, UINT16_MAX);
}

___
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/movtextenc: Check for too many styles

[Andreas Rheinhardt]

Guo, Yejun:
> 
> 
>> -Original Message-
>> From: ffmpeg-devel  On Behalf Of
>> Andreas Rheinhardt
>> Sent: 2021年2月21日 9:41
>> To: ffmpeg-devel@ffmpeg.org
>> Cc: Andreas Rheinhardt 
>> Subject: [FFmpeg-devel] [PATCH 1/2] avcodec/movtextenc: Check for too many
>> styles
>>
>> The counter for the number of styles is written on two bytes, ergo anything >
>> UINT16_MAX is invalid. This also fixes a compiler warning because of a
>> tautologically true check on 64bit systems.
>>
>> Signed-off-by: Andreas Rheinhardt 
>> ---
>> A better solution would be to error out as soon as the byte length of a 
>> subtitle
>> exceeds UINT16_MAX; yet for this one would have to modify all of ass_split to
>> allow the callbacks to return errors.
>>
>>  libavcodec/movtextenc.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c index
>> 1bef21e0b9..cf30adbd0a 100644
>> --- a/libavcodec/movtextenc.c
>> +++ b/libavcodec/movtextenc.c
>> @@ -355,7 +355,7 @@ static int mov_text_style_start(MovTextContext *s)
>>  StyleBox *tmp;
>>
>>  // last style != defaults, end the style entry and start a new one
>> -if (s->count + 1 > SIZE_MAX / sizeof(*s->style_attributes) ||
>> +if (s->count + 1 > FFMIN(SIZE_MAX /
>> + sizeof(*s->style_attributes), UINT16_MAX) ||
> 
> hi, logically, I think the result of FFMIN(SIZE_MAX / 
> sizeof(*s->style_attributes), UINT16_MAX) is always UINT16_MAX, we may just 
> use 's->count + 1 > UINT16_MAX'.
> 

And why?

> 
>>  !(tmp = av_fast_realloc(s->style_attributes,
>>
>> >style_attributes_bytes_allocated,
>>  (s->count + 1) *
>> sizeof(*s->style_attributes {
>> --
>> 2.27.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 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 1/2] avcodec/movtextenc: Check for too many styles

[Guo, Yejun]

> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Andreas Rheinhardt
> Sent: 2021年2月21日 9:41
> To: ffmpeg-devel@ffmpeg.org
> Cc: Andreas Rheinhardt 
> Subject: [FFmpeg-devel] [PATCH 1/2] avcodec/movtextenc: Check for too many
> styles
> 
> The counter for the number of styles is written on two bytes, ergo anything >
> UINT16_MAX is invalid. This also fixes a compiler warning because of a
> tautologically true check on 64bit systems.
> 
> Signed-off-by: Andreas Rheinhardt 
> ---
> A better solution would be to error out as soon as the byte length of a 
> subtitle
> exceeds UINT16_MAX; yet for this one would have to modify all of ass_split to
> allow the callbacks to return errors.
> 
>  libavcodec/movtextenc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c index
> 1bef21e0b9..cf30adbd0a 100644
> --- a/libavcodec/movtextenc.c
> +++ b/libavcodec/movtextenc.c
> @@ -355,7 +355,7 @@ static int mov_text_style_start(MovTextContext *s)
>  StyleBox *tmp;
> 
>  // last style != defaults, end the style entry and start a new one
> -if (s->count + 1 > SIZE_MAX / sizeof(*s->style_attributes) ||
> +if (s->count + 1 > FFMIN(SIZE_MAX /
> + sizeof(*s->style_attributes), UINT16_MAX) ||

hi, logically, I think the result of FFMIN(SIZE_MAX / 
sizeof(*s->style_attributes), UINT16_MAX) is always UINT16_MAX, we may just use 
's->count + 1 > UINT16_MAX'.


>  !(tmp = av_fast_realloc(s->style_attributes,
> 
> >style_attributes_bytes_allocated,
>  (s->count + 1) *
> sizeof(*s->style_attributes {
> --
> 2.27.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 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 v6 4/9] avcodec: add cbs for h266/vvc

[Nuo Mi]

On Sat, Feb 20, 2021 at 2:17 AM Jan Ekström  wrote:

> On Fri, Feb 19, 2021 at 3:52 PM Nuo Mi  wrote:
> > > >>
> > > >> The current logic is that we are writing an AU, so the first NAL
> unit in
> > > >> it is necessarily an AU start and subsequent NAL units are not?
> > > >>
> > > > H.266 AU contains one or more PU(3.105). One PU contains one coded
> > > picture.
> > > > Only the first NAL unit of an AU needs the zero_byte(B.2.2)
> > > > H.265 has no PU, each AU has one coded picture(3.1)
> > > > H.266's PU is the H.265's AU. In vvc_parser, we split bitstream to
> PU.
> > >
> > > Er, I don't think this is right: an AVPacket should contain all of the
> > > pictures for a single timestamp - a decoder can then output at most one
> > > picture from it, for the selected spatial layer.
> > >
> > This will make parser code very complicate :).
> >
> >
> > > Though I don't know how the container formats are defined here, for
> other
> > > codecs having multiple packets with the same timestamps causes ugly
> > > problems in muxing/demuxing which we avoid by saying you aren't
> allowed to
> > > do that.
> > >
> > We do not have container format spec for vvc yet. The codec is not
> register
> > on http://mp4ra.org/#/codecs. How we handle svc in H.265? It will have
> same
> > timestamp for diffrent AU.
> >
>
> FYI, there are already drafts for 14496-15 (Carriage of NAL unit based
> video in ISOBMFF aka MP4) with VVC such as document N18825 .
>
> https://isotc.iso.org/livelink/livelink?func=ll=21196449=Open=1

thanks, Jan, it's very informative.

Hi Mark,
You are right, let me spend time adjusting the code.
>each sample in the track contains an access unit. The access unit contains
NAL units from all the layers and sub-layers that are part of the operating
point.(11.3.4)

>
>
> Jan
> ___
> 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 2/8] libavfilter/vf_ssim.c: fix build warning

[Guo, Yejun]

> -Original Message-
> From: ffmpeg-devel  On Behalf Of
> Andreas Rheinhardt
> Sent: Sunday, February 21, 2021 6:21 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH 2/8] libavfilter/vf_ssim.c: fix build
> warning
> 
> Guo, Yejun:
> > The build warning message:
> > src/libavfilter/vf_ssim.c: In function ‘ssim_plane_16bit’:
> > src/libavfilter/vf_ssim.c:246:24: warning: ‘main’ is usually a function [-
> Wmain]
> >  const uint8_t *main = td->main_data[c];
> > ^~~~
> > src/libavfilter/vf_ssim.c: In function ‘ssim_plane’:
> > src/libavfilter/vf_ssim.c:289:24: warning: ‘main’ is usually a function [-
> Wmain]
> >  const uint8_t *main = td->main_data[c];
> > ^~~~
> > ---
> >  libavfilter/vf_ssim.c | 16 
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/libavfilter/vf_ssim.c b/libavfilter/vf_ssim.c index
> > 9682c093b2..ebb314c69f 100644
> > --- a/libavfilter/vf_ssim.c
> > +++ b/libavfilter/vf_ssim.c
> > @@ -243,8 +243,8 @@ static int ssim_plane_16bit(AVFilterContext *ctx,
> void *arg,
> >  const int max = td->max;
> >
> >  for (int c = 0; c < td->nb_components; c++) {
> > -const uint8_t *main = td->main_data[c];
> > -const uint8_t *ref = td->ref_data[c];
> > +const uint8_t *main_data = td->main_data[c];
> > +const uint8_t *ref_data = td->ref_data[c];
> >  const int main_stride = td->main_linesize[c];
> >  const int ref_stride = td->ref_linesize[c];
> >  int width = td->planewidth[c]; @@ -263,8 +263,8 @@ static int
> > ssim_plane_16bit(AVFilterContext *ctx, void *arg,
> >  for (int y = ystart; y < slice_end; y++) {
> >  for (; z <= y; z++) {
> >  FFSWAP(void*, sum0, sum1);
> > -ssim_4x4xn_16bit([4 * z * main_stride], main_stride,
> > - [4 * z * ref_stride], ref_stride,
> > +ssim_4x4xn_16bit(_data[4 * z * main_stride], 
> > main_stride,
> > + _data[4 * z * ref_stride],
> > + ref_stride,
> >   sum0, width);
> 
> I don't see an advantage here; it just makes the code less compact.

It's to fix the build warning. The waning says we'd change the variable name
from 'main' to other. So I change it to 'main_data', and just for a better 
'style',
I also change 'ref' to 'ref_data'.

> 
> >  }
> >
> > @@ -286,8 +286,8 @@ static int ssim_plane(AVFilterContext *ctx, void *arg,
> >  SSIMDSPContext *dsp = td->dsp;
> >
> >  for (int c = 0; c < td->nb_components; c++) {
> > -const uint8_t *main = td->main_data[c];
> > -const uint8_t *ref = td->ref_data[c];
> > +const uint8_t *main_data = td->main_data[c];
> > +const uint8_t *ref_data = td->ref_data[c];
> >  const int main_stride = td->main_linesize[c];
> >  const int ref_stride = td->ref_linesize[c];
> >  int width = td->planewidth[c]; @@ -306,8 +306,8 @@ static int
> > ssim_plane(AVFilterContext *ctx, void *arg,
> >  for (int y = ystart; y < slice_end; y++) {
> >  for (; z <= y; z++) {
> >  FFSWAP(void*, sum0, sum1);
> > -dsp->ssim_4x4_line([4 * z * main_stride], main_stride,
> > -   [4 * z * ref_stride], ref_stride,
> > +dsp->ssim_4x4_line(_data[4 * z * main_stride],
> main_stride,
> > +   _data[4 * z * ref_stride],
> > + ref_stride,
> > sum0, width);
> >  }
> >
> >
> 
> ___
> 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".

[FFmpeg-devel] [PATCH 2/2] avcodec/bitstream: Rewrite code to avoid triggering compiler warning

[Andreas Rheinhardt]

Clang infers from the existence of a default case that said case can be
taken. In case of libavcodec/bitstream.c said default case consisted of
an av_assert1 that evaluates to nothing in case of the ordinary assert
level. In this case (that doesn't happen) a variable wouldn't be
initialized, so Clang emitted Wsometimes-uninitialized warnings.
Solve this by making sure that the default path also initializes
the aforementioned variable.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/bitstream.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libavcodec/bitstream.c b/libavcodec/bitstream.c
index 7570fb2204..e425ffdc96 100644
--- a/libavcodec/bitstream.c
+++ b/libavcodec/bitstream.c
@@ -104,10 +104,10 @@ void ff_copy_bits(PutBitContext *pb, const uint8_t *src, 
int length)
 v = *(const uint16_t *)ptr; \
 break;  \
 case 4: \
+default:\
+av_assert1(size == 4);  \
 v = *(const uint32_t *)ptr; \
 break;  \
-default:\
-av_assert1(0);  \
 }   \
 }
 
-- 
2.27.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] avcodec/movtextenc: Check for too many styles

[Andreas Rheinhardt]

The counter for the number of styles is written on two bytes, ergo
anything > UINT16_MAX is invalid. This also fixes a compiler warning
because of a tautologically true check on 64bit systems.

Signed-off-by: Andreas Rheinhardt 
---
A better solution would be to error out as soon as the byte length of a
subtitle exceeds UINT16_MAX; yet for this one would have to modify all
of ass_split to allow the callbacks to return errors.

 libavcodec/movtextenc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavcodec/movtextenc.c b/libavcodec/movtextenc.c
index 1bef21e0b9..cf30adbd0a 100644
--- a/libavcodec/movtextenc.c
+++ b/libavcodec/movtextenc.c
@@ -355,7 +355,7 @@ static int mov_text_style_start(MovTextContext *s)
 StyleBox *tmp;
 
 // last style != defaults, end the style entry and start a new one
-if (s->count + 1 > SIZE_MAX / sizeof(*s->style_attributes) ||
+if (s->count + 1 > FFMIN(SIZE_MAX / sizeof(*s->style_attributes), 
UINT16_MAX) ||
 !(tmp = av_fast_realloc(s->style_attributes,
 >style_attributes_bytes_allocated,
 (s->count + 1) * 
sizeof(*s->style_attributes {
-- 
2.27.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".

Re: [FFmpeg-devel] [RFC][PATCH] avcodec: add a get_enc_buffer() callback to AVCodecContext

[James Almer]

On 2/20/2021 9:20 PM, Andreas Rheinhardt wrote:

James Almer:

On 2/20/2021 8:41 PM, Lynne wrote:

Feb 20, 2021, 21:53 by jamr...@gmail.com:


This callback is functionally the same as get_buffer2() is for
decoders, and
implements for the new encode API the functionality of the old encode
API had
where the user could provide their own buffers.

Signed-off-by: James Almer 
---
As suggested by Anton, here's get_buffer() for encoders. This way,
users of the
new encode API can still provide their own buffers, like they could
with the
old API, now that it's being removed.

The initial implementation of the default callback uses the existing
method of
simply calling av_new_packet() for it. In the future, a buffer pool
could be
used instead.
Is AV_GET_ENC_BUFFER_FLAG_REF useful here? Is there an encoder that
keeps a
reference to a previous packet around?



Could be useful, and keeps it symmetrical with decoding, so I'd keep it,
just in case.



Alternative names for the callback field and public default callback
function
are welcome, hence it being RFC.



get_enc_buffer() -> get_encoder_buffer()
avcodec_default_get_enc_buffer-> avcodec_default_get_encoder_buffer()
That's all I'd suggest. It's just 4 more characters.


If others prefer that, I'll use it.

  

+    /**
+ * This callback is called at the beginning of each packet to
get a data
+ * buffer for it.
+ *
+ * The following field will be set in the packet before this
callback is
+ * called:
+ * - size (may by zero)
+ * This callback must use the above value to calculate the
required buffer size,
+ * which must padded by at least AV_INPUT_BUFFER_PADDING_SIZE
bytes.
+ *
+ * This callback must fill the following fields in the packet:
+ * - data
+ * - buf must contain a pointer to an AVBufferRef structure. The
packet's
+ *   data pointer must be contained in it.
+ *   See: av_buffer_create(), av_buffer_alloc(), and
av_buffer_ref().



Does a size of zero mean you can have a NULL packet data and buffer
or a AV_INPUT_BUFFER_PADDING_SIZE-sized buffer?


As i mentioned on IRC, calling ff_alloc_packet2() or av_new_packet()
with size 0 is valid (it will indeed allocate an AVBufferRef of padding
size bytes, with pkt->data pointing to it and pkt->size set to 0), so a
function that replaces the former and essentially wraps the latter
should probably also allow it.

I didn't want to keep ambiguous cases in the doxy, but i can remove that
part. av_new_packet() doesn't mention it either after all.





+ *
+ * If AV_CODEC_CAP_DR1 is not set then get_enc_buffer() must call
+ * avcodec_default_get_enc_buffer() instead of providing a
buffer allocated by
+ * some other means.
+ *
+ * If AV_GET_ENC_BUFFER_FLAG_REF is set in flags then the packet
may be reused
+ * (read and/or written to if it is writable) later by libavcodec.
+ *
+ * @see avcodec_default_get_enc_buffer()
+ *
+ * - encoding: Set by libavcodec, user can override.
+ * - decoding: unused
+ */
+    int (*get_enc_buffer)(struct AVCodecContext *s, AVPacket *pkt,
int flags);



Maybe mention thread safety? Since in a frame-threaded encoder
this may be called from different threads.


So copy paste the paragraph "If frame multithreading is used, this
callback may be called from a different thread, but not from more than
one at once. Does not need to be reentrant" from get_buffer2()?.


I don't like the "not from more than one at once" part as this implies
that we would have to somehow guard ff_alloc_packet2 by a mutex, which
is currently not so.


ff_alloc_packet2() is not going to use this API as it uses this 
intermediary internal byte buffer that exists mainly for mpegvideoenc, 
so i decided to keep it independent, and any encoder adapted to use this 
callback will instead use the new function introduced in this patch.


I'll in any case use Lynne's suggestion.
___
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] [RFC][PATCH] avcodec: add a get_enc_buffer() callback to AVCodecContext

[Andreas Rheinhardt]

Lynne:
> Feb 21, 2021, 01:10 by jamr...@gmail.com:
> 
>> On 2/20/2021 8:41 PM, Lynne wrote:
>>
>>> Maybe mention thread safety? Since in a frame-threaded encoder
>>> this may be called from different threads.
>>>
>>
>> So copy paste the paragraph "If frame multithreading is used, this callback 
>> may be called from a different thread, but not from more than one at once. 
>> Does not need to be reentrant" from get_buffer2()?.
>>
> 
> No, that's the previous definition before we deprecated 
> "thread_safe_callbacks".
> So something like "The callback should be thread-safe, as when frame 
> multithreading
> is used, it may be called from multiple threads simultaneously.".

If you look at thread_get_buffer_internal, you'll see that calls to
ff_get_buffer never happen simultaneously from different threads. Even
with thread_safe_callbacks. The deprecation hasn't changed that.

- 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] [RFC][PATCH] avcodec: add a get_enc_buffer() callback to AVCodecContext

[Lynne]

Feb 21, 2021, 01:10 by jamr...@gmail.com:

> On 2/20/2021 8:41 PM, Lynne wrote:
>
>> Maybe mention thread safety? Since in a frame-threaded encoder
>> this may be called from different threads.
>>
>
> So copy paste the paragraph "If frame multithreading is used, this callback 
> may be called from a different thread, but not from more than one at once. 
> Does not need to be reentrant" from get_buffer2()?.
>

No, that's the previous definition before we deprecated "thread_safe_callbacks".
So something like "The callback should be thread-safe, as when frame 
multithreading
is used, it may be called from multiple threads simultaneously.".
___
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] [RFC][PATCH] avcodec: add a get_enc_buffer() callback to AVCodecContext

[Andreas Rheinhardt]

James Almer:
> On 2/20/2021 8:41 PM, Lynne wrote:
>> Feb 20, 2021, 21:53 by jamr...@gmail.com:
>>
>>> This callback is functionally the same as get_buffer2() is for
>>> decoders, and
>>> implements for the new encode API the functionality of the old encode
>>> API had
>>> where the user could provide their own buffers.
>>>
>>> Signed-off-by: James Almer 
>>> ---
>>> As suggested by Anton, here's get_buffer() for encoders. This way,
>>> users of the
>>> new encode API can still provide their own buffers, like they could
>>> with the
>>> old API, now that it's being removed.
>>>
>>> The initial implementation of the default callback uses the existing
>>> method of
>>> simply calling av_new_packet() for it. In the future, a buffer pool
>>> could be
>>> used instead.
>>> Is AV_GET_ENC_BUFFER_FLAG_REF useful here? Is there an encoder that
>>> keeps a
>>> reference to a previous packet around?
>>>
>>
>> Could be useful, and keeps it symmetrical with decoding, so I'd keep it,
>> just in case.
>>
>>
>>> Alternative names for the callback field and public default callback
>>> function
>>> are welcome, hence it being RFC.
>>>
>>
>> get_enc_buffer() -> get_encoder_buffer()
>> avcodec_default_get_enc_buffer-> avcodec_default_get_encoder_buffer()
>> That's all I'd suggest. It's just 4 more characters.
> 
> If others prefer that, I'll use it.
> 
>>  
>>> +    /**
>>> + * This callback is called at the beginning of each packet to
>>> get a data
>>> + * buffer for it.
>>> + *
>>> + * The following field will be set in the packet before this
>>> callback is
>>> + * called:
>>> + * - size (may by zero)
>>> + * This callback must use the above value to calculate the
>>> required buffer size,
>>> + * which must padded by at least AV_INPUT_BUFFER_PADDING_SIZE
>>> bytes.
>>> + *
>>> + * This callback must fill the following fields in the packet:
>>> + * - data
>>> + * - buf must contain a pointer to an AVBufferRef structure. The
>>> packet's
>>> + *   data pointer must be contained in it.
>>> + *   See: av_buffer_create(), av_buffer_alloc(), and
>>> av_buffer_ref().
>>>
>>
>> Does a size of zero mean you can have a NULL packet data and buffer
>> or a AV_INPUT_BUFFER_PADDING_SIZE-sized buffer?
> 
> As i mentioned on IRC, calling ff_alloc_packet2() or av_new_packet()
> with size 0 is valid (it will indeed allocate an AVBufferRef of padding
> size bytes, with pkt->data pointing to it and pkt->size set to 0), so a
> function that replaces the former and essentially wraps the latter
> should probably also allow it.
> 
> I didn't want to keep ambiguous cases in the doxy, but i can remove that
> part. av_new_packet() doesn't mention it either after all.
> 
>>
>>
>>> + *
>>> + * If AV_CODEC_CAP_DR1 is not set then get_enc_buffer() must call
>>> + * avcodec_default_get_enc_buffer() instead of providing a
>>> buffer allocated by
>>> + * some other means.
>>> + *
>>> + * If AV_GET_ENC_BUFFER_FLAG_REF is set in flags then the packet
>>> may be reused
>>> + * (read and/or written to if it is writable) later by libavcodec.
>>> + *
>>> + * @see avcodec_default_get_enc_buffer()
>>> + *
>>> + * - encoding: Set by libavcodec, user can override.
>>> + * - decoding: unused
>>> + */
>>> +    int (*get_enc_buffer)(struct AVCodecContext *s, AVPacket *pkt,
>>> int flags);
>>>
>>
>> Maybe mention thread safety? Since in a frame-threaded encoder
>> this may be called from different threads.
> 
> So copy paste the paragraph "If frame multithreading is used, this
> callback may be called from a different thread, but not from more than
> one at once. Does not need to be reentrant" from get_buffer2()?.
> 
I don't like the "not from more than one at once" part as this implies
that we would have to somehow guard ff_alloc_packet2 by a mutex, which
is currently not so.

- 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] [RFC][PATCH] avcodec: add a get_enc_buffer() callback to AVCodecContext

[James Almer]

On 2/20/2021 8:41 PM, Lynne wrote:

Feb 20, 2021, 21:53 by jamr...@gmail.com:


This callback is functionally the same as get_buffer2() is for decoders, and
implements for the new encode API the functionality of the old encode API had
where the user could provide their own buffers.

Signed-off-by: James Almer 
---
As suggested by Anton, here's get_buffer() for encoders. This way, users of the
new encode API can still provide their own buffers, like they could with the
old API, now that it's being removed.

The initial implementation of the default callback uses the existing method of
simply calling av_new_packet() for it. In the future, a buffer pool could be
used instead.
Is AV_GET_ENC_BUFFER_FLAG_REF useful here? Is there an encoder that keeps a
reference to a previous packet around?



Could be useful, and keeps it symmetrical with decoding, so I'd keep it,
just in case.



Alternative names for the callback field and public default callback function
are welcome, hence it being RFC.



get_enc_buffer() -> get_encoder_buffer()
avcodec_default_get_enc_buffer-> avcodec_default_get_encoder_buffer()
That's all I'd suggest. It's just 4 more characters.


If others prefer that, I'll use it.

  


+/**
+ * This callback is called at the beginning of each packet to get a data
+ * buffer for it.
+ *
+ * The following field will be set in the packet before this callback is
+ * called:
+ * - size (may by zero)
+ * This callback must use the above value to calculate the required buffer 
size,
+ * which must padded by at least AV_INPUT_BUFFER_PADDING_SIZE bytes.
+ *
+ * This callback must fill the following fields in the packet:
+ * - data
+ * - buf must contain a pointer to an AVBufferRef structure. The packet's
+ *   data pointer must be contained in it.
+ *   See: av_buffer_create(), av_buffer_alloc(), and av_buffer_ref().



Does a size of zero mean you can have a NULL packet data and buffer
or a AV_INPUT_BUFFER_PADDING_SIZE-sized buffer?


As i mentioned on IRC, calling ff_alloc_packet2() or av_new_packet() 
with size 0 is valid (it will indeed allocate an AVBufferRef of padding 
size bytes, with pkt->data pointing to it and pkt->size set to 0), so a 
function that replaces the former and essentially wraps the latter 
should probably also allow it.


I didn't want to keep ambiguous cases in the doxy, but i can remove that 
part. av_new_packet() doesn't mention it either after all.






+ *
+ * If AV_CODEC_CAP_DR1 is not set then get_enc_buffer() must call
+ * avcodec_default_get_enc_buffer() instead of providing a buffer 
allocated by
+ * some other means.
+ *
+ * If AV_GET_ENC_BUFFER_FLAG_REF is set in flags then the packet may be 
reused
+ * (read and/or written to if it is writable) later by libavcodec.
+ *
+ * @see avcodec_default_get_enc_buffer()
+ *
+ * - encoding: Set by libavcodec, user can override.
+ * - decoding: unused
+ */
+int (*get_enc_buffer)(struct AVCodecContext *s, AVPacket *pkt, int flags);



Maybe mention thread safety? Since in a frame-threaded encoder
this may be called from different threads.


So copy paste the paragraph "If frame multithreading is used, this 
callback may be called from a different thread, but not from more than 
one at once. Does not need to be reentrant" from get_buffer2()?.




Rest looks good.
___
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] avcodec/exr: simplify piz decompression

[Paul B Mahol]

On Sat, Feb 20, 2021 at 11:11 PM Andreas Rheinhardt <
andreas.rheinha...@gmail.com> wrote:

> Paul B Mahol:
> > Signed-off-by: Paul B Mahol 
> > ---
> >  libavcodec/exr.c | 212 +++
> >  1 file changed, 65 insertions(+), 147 deletions(-)
> >
> > diff --git a/libavcodec/exr.c b/libavcodec/exr.c
> > index cacdff5774..625ee4680c 100644
> > --- a/libavcodec/exr.c
> > +++ b/libavcodec/exr.c
> > @@ -91,6 +91,12 @@ enum ExrTileLevelRound {
> >  EXR_TILE_ROUND_UNKNOWN,
> >  };
> >
> > +typedef struct HuffEntry {
> > +uint8_t  len;
> > +uint16_t sym;
> > +uint16_t code;
>
> The old code allowed codes with a length of <= 58. This is more than our
> VLC-API allows and even more than fits into a 16-bit code. You seem to
> believe that all codes have a length <= 16 just because HUF_ENCBITS is
> 16. But this is wrong: It just means that there are at most 1<<16
> ordinary symbols and one special symbol for runs. It also means that we
> can't even distinguish all possible symbols because VLC_TYPE is 16 bits.
>

Fixed to use 32bits code, also added messages to ask for sample if more is
needed.


> > +} HuffEntry;
> > +
> >  typedef struct EXRChannel {
> >  int xsub, ysub;
> >  enum ExrPixelType pixel_type;
> > @@ -116,6 +122,11 @@ typedef struct EXRThreadData {
> >  int ysize, xsize;
> >
> >  int channel_line_size;
> > +
> > +uint16_t run_sym;
> > +HuffEntry *he;
> > +uint64_t *freq;
> > +VLC vlc;
> >  } EXRThreadData;
> >
> >  typedef struct EXRContext {
> > @@ -319,11 +330,8 @@ static void apply_lut(const uint16_t *lut, uint16_t
> *dst, int dsize)
> >  }
> >
> >  #define HUF_ENCBITS 16  // literal (value) bit length
> > -#define HUF_DECBITS 14  // decoding bit size (>= 8)
> >
> >  #define HUF_ENCSIZE ((1 << HUF_ENCBITS) + 1)  // encoding table size
> > -#define HUF_DECSIZE (1 << HUF_DECBITS)// decoding table size
> > -#define HUF_DECMASK (HUF_DECSIZE - 1)
> >
> >  typedef struct HufDec {
> >  int len;
> > @@ -336,7 +344,7 @@ static void huf_canonical_code_table(uint64_t *hcode)
> >  uint64_t c, n[59] = { 0 };
> >  int i;
> >
> > -for (i = 0; i < HUF_ENCSIZE; ++i)
> > +for (i = 0; i < HUF_ENCSIZE; i++)
>
> Spurious change.
>
> >  n[hcode[i]] += 1;
> >
> >  c = 0;
> > @@ -399,149 +407,63 @@ static int huf_unpack_enc_table(GetByteContext
> *gb,
> >  return 0;
> >  }
> >
> > -static int huf_build_dec_table(const uint64_t *hcode, int im,
> > -   int iM, HufDec *hdecod)
> > +static int huf_build_dec_table(EXRThreadData *td, int im, int iM)
> >  {
> > -for (; im <= iM; im++) {
> > -uint64_t c = hcode[im] >> 6;
> > -int i, l = hcode[im] & 63;
> > -
> > -if (c >> l)
> > -return AVERROR_INVALIDDATA;
> > -
> > -if (l > HUF_DECBITS) {
> > -HufDec *pl = hdecod + (c >> (l - HUF_DECBITS));
> > -if (pl->len)
> > -return AVERROR_INVALIDDATA;
> > -
> > -pl->lit++;
> > -
> > -pl->p = av_realloc(pl->p, pl->lit * sizeof(int));
> > -if (!pl->p)
> > -return AVERROR(ENOMEM);
> > -
> > -pl->p[pl->lit - 1] = im;
> > -} else if (l) {
> > -HufDec *pl = hdecod + (c << (HUF_DECBITS - l));
> > -
> > -for (i = 1 << (HUF_DECBITS - l); i > 0; i--, pl++) {
> > -if (pl->len || pl->p)
> > -return AVERROR_INVALIDDATA;
> > -pl->len = l;
> > -pl->lit = im;
> > -}
> > -}
> > +int j = 0;
> > +
> > +for (int i = im; i < iM; i++) {
> > +td->he[j].sym = i;
> > +td->he[j].len = td->freq[i] & 63;
> > +td->he[j].code = td->freq[i] >> 6;> +if (td->he[j].len
> > 0)
> > +j++;
> > +else
> > +td->run_sym = i;
> >  }
> >
> > -return 0;
> > -}
> > -
> > -#define get_char(c, lc, gb)
>\
> > -{
>\
> > -c   = (c << 8) | bytestream2_get_byte(gb);
>   \
> > -lc += 8;
>   \
> > -}
> > +td->he[j].sym = td->run_sym;
> > +td->he[j].len = td->freq[iM] & 63;
> > +td->he[j].code = td->freq[iM] >> 6;
> > +j++;
> >
> > -#define get_code(po, rlc, c, lc, gb, out, oe, outb)
>\
> > -{
>\
> > -if (po == rlc) {
>   \
> > -if (lc < 8)
>\
> > -get_char(c, lc, gb);
>   \
> > -lc -= 8;
>   \
> > -
>   \
> > -cs = c >> lc;
>\
> > -
>   \
> > -if (out + cs > oe || out == outb)
>\
> > -return AVERROR_INVALIDDATA;
>\
> > -
>   \
> > -s = out[-1];
>   \
> > -
>   \
> > -while (cs-- > 0)
>   \
> > -*out++ = s;
>\
> > -} else if (out < oe) {
>   \
> > -*out++ = po;
>   \
> > -} else {
>   \

[FFmpeg-devel] [PATCH] avcodec/exr: simplify piz decompression

[Paul B Mahol]

Note that >32 codes are no longer supported, give
proper error code if such scenario ever happens.

Signed-off-by: Paul B Mahol 
---
 libavcodec/exr.c | 251 +--
 1 file changed, 89 insertions(+), 162 deletions(-)

diff --git a/libavcodec/exr.c b/libavcodec/exr.c
index cacdff5774..5f99d9d5ab 100644
--- a/libavcodec/exr.c
+++ b/libavcodec/exr.c
@@ -91,6 +91,12 @@ enum ExrTileLevelRound {
 EXR_TILE_ROUND_UNKNOWN,
 };
 
+typedef struct HuffEntry {
+uint8_t  len;
+uint16_t sym;
+uint32_t code;
+} HuffEntry;
+
 typedef struct EXRChannel {
 int xsub, ysub;
 enum ExrPixelType pixel_type;
@@ -116,6 +122,11 @@ typedef struct EXRThreadData {
 int ysize, xsize;
 
 int channel_line_size;
+
+int run_sym;
+HuffEntry *he;
+uint64_t *freq;
+VLC vlc;
 } EXRThreadData;
 
 typedef struct EXRContext {
@@ -319,25 +330,15 @@ static void apply_lut(const uint16_t *lut, uint16_t *dst, 
int dsize)
 }
 
 #define HUF_ENCBITS 16  // literal (value) bit length
-#define HUF_DECBITS 14  // decoding bit size (>= 8)
-
 #define HUF_ENCSIZE ((1 << HUF_ENCBITS) + 1)  // encoding table size
-#define HUF_DECSIZE (1 << HUF_DECBITS)// decoding table size
-#define HUF_DECMASK (HUF_DECSIZE - 1)
 
-typedef struct HufDec {
-int len;
-int lit;
-int *p;
-} HufDec;
-
-static void huf_canonical_code_table(uint64_t *hcode)
+static void huf_canonical_code_table(uint64_t *freq)
 {
 uint64_t c, n[59] = { 0 };
 int i;
 
-for (i = 0; i < HUF_ENCSIZE; ++i)
-n[hcode[i]] += 1;
+for (i = 0; i < HUF_ENCSIZE; i++)
+n[freq[i]] += 1;
 
 c = 0;
 for (i = 58; i > 0; --i) {
@@ -347,10 +348,10 @@ static void huf_canonical_code_table(uint64_t *hcode)
 }
 
 for (i = 0; i < HUF_ENCSIZE; ++i) {
-int l = hcode[i];
+int l = freq[i];
 
 if (l > 0)
-hcode[i] = l | (n[l]++ << 6);
+freq[i] = l | (n[l]++ << 6);
 }
 }
 
@@ -360,7 +361,7 @@ static void huf_canonical_code_table(uint64_t *hcode)
 #define LONGEST_LONG_RUN(255 + SHORTEST_LONG_RUN)
 
 static int huf_unpack_enc_table(GetByteContext *gb,
-int32_t im, int32_t iM, uint64_t *hcode)
+int32_t im, int32_t iM, uint64_t *freq)
 {
 GetBitContext gbit;
 int ret = init_get_bits8(, gb->buffer, 
bytestream2_get_bytes_left(gb));
@@ -368,7 +369,7 @@ static int huf_unpack_enc_table(GetByteContext *gb,
 return ret;
 
 for (; im <= iM; im++) {
-uint64_t l = hcode[im] = get_bits(, 6);
+uint64_t l = freq[im] = get_bits(, 6);
 
 if (l == LONG_ZEROCODE_RUN) {
 int zerun = get_bits(, 8) + SHORTEST_LONG_RUN;
@@ -377,7 +378,7 @@ static int huf_unpack_enc_table(GetByteContext *gb,
 return AVERROR_INVALIDDATA;
 
 while (zerun--)
-hcode[im++] = 0;
+freq[im++] = 0;
 
 im--;
 } else if (l >= SHORT_ZEROCODE_RUN) {
@@ -387,161 +388,91 @@ static int huf_unpack_enc_table(GetByteContext *gb,
 return AVERROR_INVALIDDATA;
 
 while (zerun--)
-hcode[im++] = 0;
+freq[im++] = 0;
 
 im--;
 }
 }
 
 bytestream2_skip(gb, (get_bits_count() + 7) / 8);
-huf_canonical_code_table(hcode);
+huf_canonical_code_table(freq);
 
 return 0;
 }
 
-static int huf_build_dec_table(const uint64_t *hcode, int im,
-   int iM, HufDec *hdecod)
+static int huf_build_dec_table(EXRContext *s,
+   EXRThreadData *td, int im, int iM)
 {
-for (; im <= iM; im++) {
-uint64_t c = hcode[im] >> 6;
-int i, l = hcode[im] & 63;
-
-if (c >> l)
-return AVERROR_INVALIDDATA;
-
-if (l > HUF_DECBITS) {
-HufDec *pl = hdecod + (c >> (l - HUF_DECBITS));
-if (pl->len)
-return AVERROR_INVALIDDATA;
-
-pl->lit++;
-
-pl->p = av_realloc(pl->p, pl->lit * sizeof(int));
-if (!pl->p)
-return AVERROR(ENOMEM);
-
-pl->p[pl->lit - 1] = im;
-} else if (l) {
-HufDec *pl = hdecod + (c << (HUF_DECBITS - l));
-
-for (i = 1 << (HUF_DECBITS - l); i > 0; i--, pl++) {
-if (pl->len || pl->p)
-return AVERROR_INVALIDDATA;
-pl->len = l;
-pl->lit = im;
-}
+int j = 0;
+
+td->run_sym = -1;
+for (int i = im; i < iM; i++) {
+td->he[j].sym = i;
+td->he[j].len = td->freq[i] & 63;
+td->he[j].code = td->freq[i] >> 6;
+if (td->he[j].len > 32) {
+avpriv_request_sample(s->avctx, "Too big code length");
+return AVERROR_PATCHWELCOME;
 }
+if (td->he[j].len > 0)
+j++;
+else
+td->run_sym = i;
 }
 
-return 0;
-}
-

Re: [FFmpeg-devel] [PATCH] lavu/tx: support in-place FFT transforms

[Lynne]

Feb 10, 2021, 21:31 by d...@lynne.ee:

> Feb 10, 2021, 18:15 by d...@lynne.ee:
>
>> This commit adds support for in-place FFT transforms. Since our 
>> internal transforms were all in-place anyway, this only changes
>> the permutation on the input.
>>
>> Unfortunately, research papers were of no help here. All focused
>> on dry hardware implementations, where permutes are free, or on
>> software implementations where binary bloat is of no concern so
>> storing dozen times the transforms for each permutation and version
>> is not considered bad practice.
>> Still, for a pure C implementation, it's only around 28% slower
>> than the multi-megabyte FFTW3 in unaligned mode.
>>
>> Unlike a closed permutation like with PFA, split-radix FFT bit-reversals
>> contain multiple NOPs, multiple simple swaps, and a few chained swaps,
>> so regular single-loop single-state permute loops were not possible.
>> Instead, we filter out parts of the input indices which are redundant.
>> This allows for a single branch, and with some clever AVX512 asm,
>> could possibly be SIMD'd without refactoring.
>>
>> The inplace_idx array is guaranteed to never be larger than the
>> revtab array, and in practice only requires around log2(len) entries.
>>
>> The power-of-two MDCTs can be done in-place as well. And it's
>> possible to eliminate a copy in the compound MDCTs too, however
>> it'll be slower than doing them out of place, and we'd need to dirty
>> the input array.
>>
>> Patch attached.
>>
>
> Locally added APIchanges and lavu minor bump.
> And got rid of the unused set temporary variables when permuting.
>

Will push this tomorrow if there are no objections.
___
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] [RFC][PATCH] avcodec: add a get_enc_buffer() callback to AVCodecContext

[Lynne]

Feb 20, 2021, 21:53 by jamr...@gmail.com:

> This callback is functionally the same as get_buffer2() is for decoders, and
> implements for the new encode API the functionality of the old encode API had
> where the user could provide their own buffers.
>
> Signed-off-by: James Almer 
> ---
> As suggested by Anton, here's get_buffer() for encoders. This way, users of 
> the
> new encode API can still provide their own buffers, like they could with the
> old API, now that it's being removed.
>
> The initial implementation of the default callback uses the existing method of
> simply calling av_new_packet() for it. In the future, a buffer pool could be
> used instead.
> Is AV_GET_ENC_BUFFER_FLAG_REF useful here? Is there an encoder that keeps a
> reference to a previous packet around?
>

Could be useful, and keeps it symmetrical with decoding, so I'd keep it,
just in case.


> Alternative names for the callback field and public default callback function
> are welcome, hence it being RFC.
>

get_enc_buffer() -> get_encoder_buffer()
avcodec_default_get_enc_buffer-> avcodec_default_get_encoder_buffer()
That's all I'd suggest. It's just 4 more characters.
 

> +/**
> + * This callback is called at the beginning of each packet to get a data
> + * buffer for it.
> + *
> + * The following field will be set in the packet before this callback is
> + * called:
> + * - size (may by zero)
> + * This callback must use the above value to calculate the required 
> buffer size,
> + * which must padded by at least AV_INPUT_BUFFER_PADDING_SIZE bytes.
> + *
> + * This callback must fill the following fields in the packet:
> + * - data
> + * - buf must contain a pointer to an AVBufferRef structure. The packet's
> + *   data pointer must be contained in it.
> + *   See: av_buffer_create(), av_buffer_alloc(), and av_buffer_ref().
>

Does a size of zero mean you can have a NULL packet data and buffer
or a AV_INPUT_BUFFER_PADDING_SIZE-sized buffer?


> + *
> + * If AV_CODEC_CAP_DR1 is not set then get_enc_buffer() must call
> + * avcodec_default_get_enc_buffer() instead of providing a buffer 
> allocated by
> + * some other means.
> + *
> + * If AV_GET_ENC_BUFFER_FLAG_REF is set in flags then the packet may be 
> reused
> + * (read and/or written to if it is writable) later by libavcodec.
> + *
> + * @see avcodec_default_get_enc_buffer()
> + *
> + * - encoding: Set by libavcodec, user can override.
> + * - decoding: unused
> + */
> +int (*get_enc_buffer)(struct AVCodecContext *s, AVPacket *pkt, int 
> flags);
>

Maybe mention thread safety? Since in a frame-threaded encoder
this may be called from different threads.

Rest looks good.
___
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] libavfilter: build failure in vf_lensfun.c

[Paul B Mahol]

Only latest version of library is actually supported.

On Sat, Feb 20, 2021 at 5:51 AM Hanspeter Niederstrasser <
niederstras...@gmail.com> wrote:

> On macOS 10.13,libavfilter/vf_lensfun.c fails like this:
>
> clang -I. -Isrc/ -D_ISOC99_SOURCE -D_FILE_OFFSET_BITS=64
> -D_LARGEFILE_SOURCE -Isrc/compat/dispatch_semaphore -DPIC -DZLIB_CONST
> -DHAVE_AV_CONFIG_H -DBUILDING_avfilter -I/sw/include -std=c11
> -Werror=partial-availability -fomit-frame-pointer -fPIC -pthread
> -I/sw/include -I/sw/include -I/sw/include/bs2b -I/sw/include
> -I/sw/include -I/sw/include/freetype2 -I/sw/include/fribidi
> -I/sw/include -I/sw/include -I/sw/include -I/sw/include/lensfun
> -I/sw/include/glib-2.0 -I/sw/lib/glib-2.0/include -I/sw/include
> -I/sw/include/openjpeg-2.1 -I/sw/include -I/sw/include/opus
> -I/sw/include/opus -I/sw/include -I/sw/include -I/sw/include
> -I/sw/include -I/sw/include -I/sw/include -I/sw/include -I/sw/include
> -I/sw/include -I/sw/include -I/sw/include -I/sw/include -I/sw/include
> -I/sw/include -I/sw/include -I/sw/include -I/sw/include -I/sw/include
> -I/sw/lib/libcdio-2.1/include -I/opt/X11/include -I/opt/X11/include
> -I/opt/X11/include -I/opt/X11/include -g -Wdeclaration-after-statement
> -Wall -Wdisabled-optimization -Wpointer-arith -Wredundant-decls
> -Wwrite-strings -Wtype-limits -Wundef -Wmissing-prototypes
> -Wno-pointer-to-int-cast -Wstrict-prototypes -Wempty-body
> -Wno-parentheses -Wno-switch -Wno-format-zero-length -Wno-pointer-sign
> -Wno-unused-const-variable -Wno-char-subscripts -O3 -fno-math-errno
> -fno-signed-zeros -mstack-alignment=16 -Qunused-arguments
> -Werror=implicit-function-declaration -Werror=missing-prototypes
> -Werror=return-type -I/sw/include/SDL2 -D_THREAD_SAFE   -MMD -MF
> libavfilter/vf_lensfun.d -MT libavfilter/vf_lensfun.o -c -o
> libavfilter/vf_lensfun.o src/libavfilter/vf_lensfun.c
> In file included from src/libavfilter/vf_lensfun.c:40:
> /sw/include/lensfun/lensfun.h:252:33: warning: this function
> declaration is not a prototype [-Wstrict-prototypes]
> LF_EXPORT lfMount *lf_mount_new ();
> ^
>  void
> /sw/include/lensfun/lensfun.h:397:35: warning: this function
> declaration is not a prototype [-Wstrict-prototypes]
> LF_EXPORT lfCamera *lf_camera_new ();
>   ^
>void
> /sw/include/lensfun/lensfun.h:1177:31: warning: this function
> declaration is not a prototype [-Wstrict-prototypes]
> LF_EXPORT lfLens *lf_lens_new ();
>   ^
>void
> src/libavfilter/vf_lensfun.c:139:10: error: implicit declaration of
> function 'lf_db_create' is invalid in C99
>   [-Werror,-Wimplicit-function-declaration]
> db = lf_db_create();
>  ^
>
> There are a bunch of other implicit declaration errors against several
> other lf_* functions. Furthermore, there is another error:
> ```
> src/libavfilter/vf_lensfun.c:181:59: error: too many arguments to
> function call, expected 3, have 5
> lenses = lf_db_find_lenses(db, lensfun->camera, NULL,
> lensfun->lens_model, 0);
> ```
>
> I run configure with this:
> ```
> ../configure --prefix=/usr/local/ffmpeg-clang --enable-autodetect
> --enable-gpl --enable-version3 --disable-static --enable-shared  of other --enable-libFOO> --enable-liblensfun
> ```
>
> I have lensfun-0.3.1 installed and none of those functions are
> declared in lensfun.h. Should there be a versioned check for lensfun
> in configure or a check for other functions besides lf_db_new ?
>
> Hanspeter
>
> --
> ___
> 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 2/8] libavfilter/vf_ssim.c: fix build warning

[Andreas Rheinhardt]

Guo, Yejun:
> The build warning message:
> src/libavfilter/vf_ssim.c: In function ‘ssim_plane_16bit’:
> src/libavfilter/vf_ssim.c:246:24: warning: ‘main’ is usually a function 
> [-Wmain]
>  const uint8_t *main = td->main_data[c];
> ^~~~
> src/libavfilter/vf_ssim.c: In function ‘ssim_plane’:
> src/libavfilter/vf_ssim.c:289:24: warning: ‘main’ is usually a function 
> [-Wmain]
>  const uint8_t *main = td->main_data[c];
> ^~~~
> ---
>  libavfilter/vf_ssim.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/libavfilter/vf_ssim.c b/libavfilter/vf_ssim.c
> index 9682c093b2..ebb314c69f 100644
> --- a/libavfilter/vf_ssim.c
> +++ b/libavfilter/vf_ssim.c
> @@ -243,8 +243,8 @@ static int ssim_plane_16bit(AVFilterContext *ctx, void 
> *arg,
>  const int max = td->max;
>  
>  for (int c = 0; c < td->nb_components; c++) {
> -const uint8_t *main = td->main_data[c];
> -const uint8_t *ref = td->ref_data[c];
> +const uint8_t *main_data = td->main_data[c];
> +const uint8_t *ref_data = td->ref_data[c];
>  const int main_stride = td->main_linesize[c];
>  const int ref_stride = td->ref_linesize[c];
>  int width = td->planewidth[c];
> @@ -263,8 +263,8 @@ static int ssim_plane_16bit(AVFilterContext *ctx, void 
> *arg,
>  for (int y = ystart; y < slice_end; y++) {
>  for (; z <= y; z++) {
>  FFSWAP(void*, sum0, sum1);
> -ssim_4x4xn_16bit([4 * z * main_stride], main_stride,
> - [4 * z * ref_stride], ref_stride,
> +ssim_4x4xn_16bit(_data[4 * z * main_stride], 
> main_stride,
> + _data[4 * z * ref_stride], ref_stride,
>   sum0, width);

I don't see an advantage here; it just makes the code less compact.

>  }
>  
> @@ -286,8 +286,8 @@ static int ssim_plane(AVFilterContext *ctx, void *arg,
>  SSIMDSPContext *dsp = td->dsp;
>  
>  for (int c = 0; c < td->nb_components; c++) {
> -const uint8_t *main = td->main_data[c];
> -const uint8_t *ref = td->ref_data[c];
> +const uint8_t *main_data = td->main_data[c];
> +const uint8_t *ref_data = td->ref_data[c];
>  const int main_stride = td->main_linesize[c];
>  const int ref_stride = td->ref_linesize[c];
>  int width = td->planewidth[c];
> @@ -306,8 +306,8 @@ static int ssim_plane(AVFilterContext *ctx, void *arg,
>  for (int y = ystart; y < slice_end; y++) {
>  for (; z <= y; z++) {
>  FFSWAP(void*, sum0, sum1);
> -dsp->ssim_4x4_line([4 * z * main_stride], main_stride,
> -   [4 * z * ref_stride], ref_stride,
> +dsp->ssim_4x4_line(_data[4 * z * main_stride], 
> main_stride,
> +   _data[4 * z * ref_stride], ref_stride,
> sum0, width);
>  }
>  
> 

___
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/exr: simplify piz decompression

[Andreas Rheinhardt]

Paul B Mahol:
> Signed-off-by: Paul B Mahol 
> ---
>  libavcodec/exr.c | 212 +++
>  1 file changed, 65 insertions(+), 147 deletions(-)
> 
> diff --git a/libavcodec/exr.c b/libavcodec/exr.c
> index cacdff5774..625ee4680c 100644
> --- a/libavcodec/exr.c
> +++ b/libavcodec/exr.c
> @@ -91,6 +91,12 @@ enum ExrTileLevelRound {
>  EXR_TILE_ROUND_UNKNOWN,
>  };
>  
> +typedef struct HuffEntry {
> +uint8_t  len;
> +uint16_t sym;
> +uint16_t code;

The old code allowed codes with a length of <= 58. This is more than our
VLC-API allows and even more than fits into a 16-bit code. You seem to
believe that all codes have a length <= 16 just because HUF_ENCBITS is
16. But this is wrong: It just means that there are at most 1<<16
ordinary symbols and one special symbol for runs. It also means that we
can't even distinguish all possible symbols because VLC_TYPE is 16 bits.

> +} HuffEntry;
> +
>  typedef struct EXRChannel {
>  int xsub, ysub;
>  enum ExrPixelType pixel_type;
> @@ -116,6 +122,11 @@ typedef struct EXRThreadData {
>  int ysize, xsize;
>  
>  int channel_line_size;
> +
> +uint16_t run_sym;
> +HuffEntry *he;
> +uint64_t *freq;
> +VLC vlc;
>  } EXRThreadData;
>  
>  typedef struct EXRContext {
> @@ -319,11 +330,8 @@ static void apply_lut(const uint16_t *lut, uint16_t 
> *dst, int dsize)
>  }
>  
>  #define HUF_ENCBITS 16  // literal (value) bit length
> -#define HUF_DECBITS 14  // decoding bit size (>= 8)
>  
>  #define HUF_ENCSIZE ((1 << HUF_ENCBITS) + 1)  // encoding table size
> -#define HUF_DECSIZE (1 << HUF_DECBITS)// decoding table size
> -#define HUF_DECMASK (HUF_DECSIZE - 1)
>  
>  typedef struct HufDec {
>  int len;
> @@ -336,7 +344,7 @@ static void huf_canonical_code_table(uint64_t *hcode)
>  uint64_t c, n[59] = { 0 };
>  int i;
>  
> -for (i = 0; i < HUF_ENCSIZE; ++i)
> +for (i = 0; i < HUF_ENCSIZE; i++)

Spurious change.

>  n[hcode[i]] += 1;
>  
>  c = 0;
> @@ -399,149 +407,63 @@ static int huf_unpack_enc_table(GetByteContext *gb,
>  return 0;
>  }
>  
> -static int huf_build_dec_table(const uint64_t *hcode, int im,
> -   int iM, HufDec *hdecod)
> +static int huf_build_dec_table(EXRThreadData *td, int im, int iM)
>  {
> -for (; im <= iM; im++) {
> -uint64_t c = hcode[im] >> 6;
> -int i, l = hcode[im] & 63;
> -
> -if (c >> l)
> -return AVERROR_INVALIDDATA;
> -
> -if (l > HUF_DECBITS) {
> -HufDec *pl = hdecod + (c >> (l - HUF_DECBITS));
> -if (pl->len)
> -return AVERROR_INVALIDDATA;
> -
> -pl->lit++;
> -
> -pl->p = av_realloc(pl->p, pl->lit * sizeof(int));
> -if (!pl->p)
> -return AVERROR(ENOMEM);
> -
> -pl->p[pl->lit - 1] = im;
> -} else if (l) {
> -HufDec *pl = hdecod + (c << (HUF_DECBITS - l));
> -
> -for (i = 1 << (HUF_DECBITS - l); i > 0; i--, pl++) {
> -if (pl->len || pl->p)
> -return AVERROR_INVALIDDATA;
> -pl->len = l;
> -pl->lit = im;
> -}
> -}
> +int j = 0;
> +
> +for (int i = im; i < iM; i++) {
> +td->he[j].sym = i;
> +td->he[j].len = td->freq[i] & 63;
> +td->he[j].code = td->freq[i] >> 6;> +if (td->he[j].len > 0)
> +j++;
> +else
> +td->run_sym = i;
>  }
>  
> -return 0;
> -}
> -
> -#define get_char(c, lc, gb)  
>  \
> -{
>  \
> -c   = (c << 8) | bytestream2_get_byte(gb);   
>  \
> -lc += 8; 
>  \
> -}
> +td->he[j].sym = td->run_sym;
> +td->he[j].len = td->freq[iM] & 63;
> +td->he[j].code = td->freq[iM] >> 6;
> +j++;
>  
> -#define get_code(po, rlc, c, lc, gb, out, oe, outb)  
>  \
> -{
>  \
> -if (po == rlc) { 
>  \
> -if (lc < 8)  
>  \
> -get_char(c, lc, gb); 
>  \
> -lc -= 8; 
>  \
> - 
>  \
> -cs = c >> lc;
>  \
> - 
>  \
> -if (out + cs > oe || out == outb)
>  \
> -return AVERROR_INVALIDDATA;  
>  \

Re: [FFmpeg-devel] [RFC] Event loop

[Mark Thompson]

On 19/02/2021 19:34, Kieran Kunhya wrote:

On Fri, 19 Feb 2021 at 19:04, Nicolas George  wrote:


Kieran Kunhya (12021-02-19):

I don't have a strong opinion on either. But I think you can use
container_of on the fd.


Thanks. I find no trace of it in the docs:


http://docs.libuv.org/en/v1.x/search.html?q=container_of_keywords=yes=default

Can you be a little more precise?

The portability arguments have been compelling for libuv over libev, so
if we find a way of getting libuv to work for our needs, it would be
best.



Actually maybe you can't use container_of and mmap since the fd is not a
pointer...


I'm not entirely sure where this discussion was going, but I believe the 
original questions would have been simple to answer if anyone had bothered to 
read the documentation.

Adding a file descriptor referring to something independent of libuv (such as an alsa 
or xcb fd) is handled by uv_poll_init(), see 
.

A user context pointer is carried into callbacks via the "data" member of uv_handle_t 
(and hence the same member in all subtypes of it, like uv_poll_t), see 
.


I think we need to be clear here that libuv and libev are solving some of the 
same problems but not really in the same way.

libev is essentially super-poll(): it gives you a uniform interface to whatever 
the preferred synchronous multiplexing method is on your platform (epoll on 
Linux, kqueue on BSD and derivatives, traditional BSD select on Windows, etc.). 
 For Windows, that of course only supports things to which the synchronous 
multiplexing interface applies, which is network sockets only.

libev isn't really giving you anything which we don't have already, though the 
common interface may be nice for other applications which also use libev.

libuv is trying to solve event loop problem in a uniform way across all 
platforms, with no requirement that the underlying method is synchronous.

That lets it provide better support on systems which want to use asynchrous I/O 
(like Windows with I/O completion ports), but has the consequence that it has 
to own the sockets internally because the details of the multiplexer affect all 
socket I/O.  If the underlying method is asynchrous, then every socket call 
will need additional detail that the user doesn't want to deal with (e.g. 
send() becomes WSASend() with an OVERLAPPED argument on Windows so the result 
can be delivered to an I/O completion port later).


In my opinion, we would be best off ensuring that we support the more general 
solution in libuv (or, if libuv has separate problems which rule it out, 
another library with the same approach such as libevent), since FFmpeg provides 
cross-platform libraries which we would like to work properly on all systems we 
support.  This has future benefits for the systems which in current form would 
be covered by libev, as asynchronous I/O becomes is becoming more common on 
Unix platforms as well (e.g. io_uring on Linux) and we will be able to handle 
it without another redesign.

Thanks,

- Mark
___
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] cbs_h265: Detect more reference combinations which would overflow the DPB

[Mark Thompson]

On 03/02/2021 21:34, Mark Thompson wrote:

In total, the number of short term references (from the selected short
term ref pic set), the number of long term references (combining both the
used candidates from the SPS and those defined in the slice header) and
the number of instances of the current picture (usually one, but can be
two if current picture reference is enabled) must never exceed the size
of the DPB.  This is a generalisation of the condition associated with
num_long_term_pics in 7.4.7.1.

We use this to apply tighter bounds to the number of long term pictures
referred to in the slice header, and also to detect the invalid case where
the second reference to the current picture would not fit in the DPB (this
case can't be detected earlier because an STRPS with 15 pictures can still
be valid in the same stream when used with a different PPS which does not
require two DPB slots for the current picture).
---
Michael: does this fix your fuzz case for num_long_term_sps?

Thanks,

- Mark


  libavcodec/cbs_h265_syntax_template.c | 23 +--
  1 file changed, 21 insertions(+), 2 deletions(-)

diff --git a/libavcodec/cbs_h265_syntax_template.c 
b/libavcodec/cbs_h265_syntax_template.c
index d09934cfeb..5d216aad36 100644
--- a/libavcodec/cbs_h265_syntax_template.c
+++ b/libavcodec/cbs_h265_syntax_template.c
@@ -1369,6 +1369,7 @@ static int 
FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw,
  if (current->nal_unit_header.nal_unit_type != HEVC_NAL_IDR_W_RADL &&
  current->nal_unit_header.nal_unit_type != HEVC_NAL_IDR_N_LP) {
  const H265RawSTRefPicSet *rps;
+    int dpb_slots_remaining;

  ub(sps->log2_max_pic_order_cnt_lsb_minus4 + 4, 
slice_pic_order_cnt_lsb);

@@ -1387,6 +1388,22 @@ static int 
FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw,
  rps = >st_ref_pic_set[0];
  }

+    dpb_slots_remaining = HEVC_MAX_DPB_SIZE - 1 -
+    rps->num_negative_pics - rps->num_positive_pics;
+    if (pps->pps_curr_pic_ref_enabled_flag &&
+    (sps->sample_adaptive_offset_enabled_flag ||
+ !pps->pps_deblocking_filter_disabled_flag ||
+ pps->deblocking_filter_override_enabled_flag)) {
+    // This picture will occupy two DPB slots.
+    if (dpb_slots_remaining == 0) {
+    av_log(ctx->log_ctx, AV_LOG_ERROR, "Invalid stream: "
+   "short-term ref pic set contains too many pictures "
+   "to use with current picture reference enabled.\n");
+    return AVERROR_INVALIDDATA;
+    }
+    --dpb_slots_remaining;
+    }
+
  num_pic_total_curr = 0;
  for (i = 0; i < rps->num_negative_pics; i++)
  if (rps->used_by_curr_pic_s0_flag[i])
@@ -1399,13 +1416,15 @@ static int 
FUNC(slice_segment_header)(CodedBitstreamContext *ctx, RWContext *rw,
  unsigned int idx_size;

  if (sps->num_long_term_ref_pics_sps > 0) {
-    ue(num_long_term_sps, 0, sps->num_long_term_ref_pics_sps);
+    ue(num_long_term_sps, 0, 
FFMIN(sps->num_long_term_ref_pics_sps,
+   dpb_slots_remaining));
  idx_size = av_log2(sps->num_long_term_ref_pics_sps - 1) + 
1;
+    dpb_slots_remaining -= current->num_long_term_sps;
  } else {
  infer(num_long_term_sps, 0);
  idx_size = 0;
  }
-    ue(num_long_term_pics, 0, HEVC_MAX_REFS - 
current->num_long_term_sps);
+    ue(num_long_term_pics, 0, dpb_slots_remaining);

  for (i = 0; i < current->num_long_term_sps +
  current->num_long_term_pics; i++) {


Ping.

- Mark
___
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] [RFC][PATCH] avcodec: add a get_enc_buffer() callback to AVCodecContext

[James Almer]

This callback is functionally the same as get_buffer2() is for decoders, and
implements for the new encode API the functionality of the old encode API had
where the user could provide their own buffers.

Signed-off-by: James Almer 
---
As suggested by Anton, here's get_buffer() for encoders. This way, users of the
new encode API can still provide their own buffers, like they could with the
old API, now that it's being removed.

The initial implementation of the default callback uses the existing method of
simply calling av_new_packet() for it. In the future, a buffer pool could be
used instead.
Is AV_GET_ENC_BUFFER_FLAG_REF useful here? Is there an encoder that keeps a
reference to a previous packet around?

Alternative names for the callback field and public default callback function
are welcome, hence it being RFC.

 libavcodec/avcodec.h | 42 ++
 libavcodec/codec.h   |  8 ---
 libavcodec/encode.c  | 54 +++-
 libavcodec/encode.h  |  7 ++
 libavcodec/options.c |  1 +
 5 files changed, 108 insertions(+), 4 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 7dbf083a24..5b4a731e9f 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -513,6 +513,11 @@ typedef struct AVProducerReferenceTime {
  */
 #define AV_GET_BUFFER_FLAG_REF (1 << 0)
 
+/**
+ * The encoder will keep a reference to the packet and may reuse it later.
+ */
+#define AV_GET_ENC_BUFFER_FLAG_REF (1 << 0)
+
 struct AVCodecInternal;
 
 /**
@@ -2346,6 +2351,36 @@ typedef struct AVCodecContext {
  * - encoding: set by user
  */
 int export_side_data;
+
+/**
+ * This callback is called at the beginning of each packet to get a data
+ * buffer for it.
+ *
+ * The following field will be set in the packet before this callback is
+ * called:
+ * - size (may by zero)
+ * This callback must use the above value to calculate the required buffer 
size,
+ * which must padded by at least AV_INPUT_BUFFER_PADDING_SIZE bytes.
+ *
+ * This callback must fill the following fields in the packet:
+ * - data
+ * - buf must contain a pointer to an AVBufferRef structure. The packet's
+ *   data pointer must be contained in it.
+ *   See: av_buffer_create(), av_buffer_alloc(), and av_buffer_ref().
+ *
+ * If AV_CODEC_CAP_DR1 is not set then get_enc_buffer() must call
+ * avcodec_default_get_enc_buffer() instead of providing a buffer 
allocated by
+ * some other means.
+ *
+ * If AV_GET_ENC_BUFFER_FLAG_REF is set in flags then the packet may be 
reused
+ * (read and/or written to if it is writable) later by libavcodec.
+ *
+ * @see avcodec_default_get_enc_buffer()
+ *
+ * - encoding: Set by libavcodec, user can override.
+ * - decoding: unused
+ */
+int (*get_enc_buffer)(struct AVCodecContext *s, AVPacket *pkt, int flags);
 } AVCodecContext;
 
 #if FF_API_CODEC_GET_SET
@@ -2920,6 +2955,13 @@ void avsubtitle_free(AVSubtitle *sub);
  */
 int avcodec_default_get_buffer2(AVCodecContext *s, AVFrame *frame, int flags);
 
+/**
+ * The default callback for AVCodecContext.get_enc_buffer(). It is made public 
so
+ * it can be called by custom get_enc_buffer() implementations for encoders 
without
+ * AV_CODEC_CAP_DR1 set.
+ */
+int avcodec_default_get_enc_buffer(AVCodecContext *s, AVPacket *pkt, int 
flags);
+
 /**
  * Modify width and height values so that they will result in a memory
  * buffer that is acceptable for the codec if you do not use any horizontal
diff --git a/libavcodec/codec.h b/libavcodec/codec.h
index 0ccbf0eb19..c3460e82ac 100644
--- a/libavcodec/codec.h
+++ b/libavcodec/codec.h
@@ -43,9 +43,11 @@
  */
 #define AV_CODEC_CAP_DRAW_HORIZ_BAND (1 <<  0)
 /**
- * Codec uses get_buffer() for allocating buffers and supports custom 
allocators.
- * If not set, it might not use get_buffer() at all or use operations that
- * assume the buffer was allocated by avcodec_default_get_buffer.
+ * Codec uses get_buffer() or get_enc_buffer() for allocating buffers and
+ * supports custom allocators.
+ * If not set, it might not use get_buffer() or get_enc_buffer() at all, or
+ * use operations that assume the buffer was allocated by
+ * avcodec_default_get_buffer2 or avcodec_default_get_enc_buffer.
  */
 #define AV_CODEC_CAP_DR1 (1 <<  1)
 #define AV_CODEC_CAP_TRUNCATED   (1 <<  3)
diff --git a/libavcodec/encode.c b/libavcodec/encode.c
index 282337e453..f464ad66b2 100644
--- a/libavcodec/encode.c
+++ b/libavcodec/encode.c
@@ -56,6 +56,52 @@ int ff_alloc_packet2(AVCodecContext *avctx, AVPacket *avpkt, 
int64_t size, int64
 return 0;
 }
 
+int avcodec_default_get_enc_buffer(AVCodecContext *avctx, AVPacket *avpkt, int 
flags)
+{
+int ret;
+
+if (avpkt->data || avpkt->buf) {
+av_log(avctx, AV_LOG_ERROR, "avpkt->{data,buf} != NULL in 
avcodec_default_get_enc_buffer()\n");
+return 

Re: [FFmpeg-devel] [PATCH] cbs_sei: Detect payload overflows when reading SEI messages

[Mark Thompson]

On 02/02/2021 20:58, Mark Thompson wrote:

The top-level GetBitContext is sized for the whole NAL unit, so it fails
to detect overflows where a payload continues into the following message.
To fix that, we make a new context on the stack for reading each payload.
---
On 01/02/2021 22:31, Michael Niedermayer wrote:

Fixes: Timeout
Fixes: 
29892/clusterfuzz-testcase-minimized-ffmpeg_BSF_H264_REDUNDANT_PPS_fuzzer-6310830956216320

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

diff --git a/libavcodec/cbs_sei_syntax_template.c 
b/libavcodec/cbs_sei_syntax_template.c
index 9114e61ff6..3b9bc942f5 100644
--- a/libavcodec/cbs_sei_syntax_template.c
+++ b/libavcodec/cbs_sei_syntax_template.c
@@ -178,6 +178,8 @@ static int FUNC(message)(CodedBitstreamContext *ctx, 
RWContext *rw,
  GetBitContext tmp = *rw;
  int trailing_bits, trailing_zero_bits;
+    if (8 * current->payload_size < bits_written)
+    return AVERROR_INVALIDDATA;
  bits_left = 8 * current->payload_size - bits_written;
  if (bits_left > 8)
  skip_bits_long(, bits_left - 8);


So it looks like the actual problem is that we don't detect payload overflow, 
so the calculation here underflows if the payload is invalid such that we read 
more bits than there actually are.

How about this answer, which tries to fix the general problem by detecting 
overflow properly - does it fix your fuzzed case?

Thanks,

- Mark


  libavcodec/cbs_sei_syntax_template.c | 17 -
  1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/libavcodec/cbs_sei_syntax_template.c 
b/libavcodec/cbs_sei_syntax_template.c
index 9114e61ff6..0ef7b42ed9 100644
--- a/libavcodec/cbs_sei_syntax_template.c
+++ b/libavcodec/cbs_sei_syntax_template.c
@@ -238,6 +238,7 @@ static int FUNC(message_list)(CodedBitstreamContext *ctx, 
RWContext *rw,
  uint32_t payload_type = 0;
  uint32_t payload_size = 0;
  uint32_t tmp;
+    GetBitContext payload_gbc;

  while (show_bits(rw, 8) == 0xff) {
  fixed(8, ff_byte, 0xff);
@@ -253,13 +254,27 @@ static int FUNC(message_list)(CodedBitstreamContext *ctx, 
RWContext *rw,
  xu(8, last_payload_size_byte, tmp, 0, 254, 0);
  payload_size += tmp;

+    // There must be space remaining for both the payload and
+    // the trailing bits on the SEI NAL unit.
+    if (payload_size + 1 > get_bits_left(rw) / 8) {
+    av_log(ctx->log_ctx, AV_LOG_ERROR,
+   "Invalid SEI message: payload_size too large "
+   "(%"PRIu32" bytes).\n", payload_size);
+    return AVERROR_INVALIDDATA;
+    }
+    CHECK(init_get_bits(_gbc, rw->buffer,
+    get_bits_count(rw) + 8 * payload_size));
+    skip_bits_long(_gbc, get_bits_count(rw));
+
  CHECK(ff_cbs_sei_list_add(current));
  message = >messages[k];

  message->payload_type = payload_type;
  message->payload_size = payload_size;

-    CHECK(FUNC(message)(ctx, rw, message));
+    CHECK(FUNC(message)(ctx, _gbc, message));
+
+    skip_bits_long(rw, 8 * payload_size);

  if (!cbs_h2645_read_more_rbsp_data(rw))
  break;


Ping.

- Mark
___
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 v2] avcodec/gifenc: Add global_palette option

[Marton Balint]

On Sat, 20 Feb 2021, Derek Buitenhuis wrote:


This option will disable the writing of the global palette in global
GIF header if it is set to 0, causing only the frame-level palette
to ever be written.

This will be useful later on when further frame-level palette
optimizations are introduced.

The default is 1, which maintains current default behavior.

Signed-off-by: Derek Buitenhuis 
---
Does this need a avcodec minor/patch version bump?
---
libavcodec/gif.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)


docs update is missing for the new option.

Regards,
Marton



diff --git a/libavcodec/gif.c b/libavcodec/gif.c
index de41992851..8c07ee2769 100644
--- a/libavcodec/gif.c
+++ b/libavcodec/gif.c
@@ -51,6 +51,7 @@ typedef struct GIFContext {
AVFrame *last_frame;
int flags;
int image;
+int use_global_palette;
uint32_t palette[AVPALETTE_COUNT];  ///< local reference palette for !pal8
int palette_loaded;
int transparent_index;
@@ -293,12 +294,14 @@ static int gif_image_write_image(AVCodecContext *avctx,

bcid = get_palette_transparency_index(global_palette);

-bytestream_put_byte(bytestream, 0xf7); /* flags: global clut, 256 
entries */
+bytestream_put_byte(bytestream, ((uint8_t) s->use_global_palette << 7) | 
0x70 | (s->use_global_palette ? 7 : 0)); /* flags: global clut, 256 entries */
bytestream_put_byte(bytestream, bcid < 0 ? DEFAULT_TRANSPARENCY_INDEX : 
bcid); /* background color index */
bytestream_put_byte(bytestream, aspect);
-for (int i = 0; i < 256; i++) {
-const uint32_t v = global_palette[i] & 0xff;
-bytestream_put_be24(bytestream, v);
+if (s->use_global_palette) {
+for (int i = 0; i < 256; i++) {
+const uint32_t v = global_palette[i] & 0xff;
+bytestream_put_be24(bytestream, v);
+}
}
}

@@ -330,15 +333,16 @@ static int gif_image_write_image(AVCodecContext *avctx,
bytestream_put_le16(bytestream, width);
bytestream_put_le16(bytestream, height);

-if (!palette) {
-bytestream_put_byte(bytestream, 0x00); /* flags */
-} else {
+if (palette || !s->use_global_palette) {
+const uint32_t *pal = palette ? palette : s->palette;
unsigned i;
bytestream_put_byte(bytestream, 1<<7 | 0x7); /* flags */
for (i = 0; i < AVPALETTE_COUNT; i++) {
-const uint32_t v = palette[i];
+const uint32_t v = pal[i];
bytestream_put_be24(bytestream, v);
}
+} else {
+bytestream_put_byte(bytestream, 0x00); /* flags */
}

bytestream_put_byte(bytestream, 0x08);
@@ -473,6 +477,7 @@ static const AVOption gif_options[] = {
{ "offsetting", "enable picture offsetting", 0, AV_OPT_TYPE_CONST, 
{.i64=GF_OFFSETTING}, INT_MIN, INT_MAX, FLAGS, "flags" },
{ "transdiff", "enable transparency detection between frames", 0, 
AV_OPT_TYPE_CONST, {.i64=GF_TRANSDIFF}, INT_MIN, INT_MAX, FLAGS, "flags" },
{ "gifimage", "enable encoding only images per frame", OFFSET(image), 
AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS },
+{ "global_palette", "write a palette to the global gif header where 
feasible", OFFSET(use_global_palette), AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, FLAGS },
{ NULL }
};

--
2.30.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 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 v2] avcodec/gifenc: Add global_palette option

[Derek Buitenhuis]

This option will disable the writing of the global palette in global
GIF header if it is set to 0, causing only the frame-level palette
to ever be written.

This will be useful later on when further frame-level palette
optimizations are introduced.

The default is 1, which maintains current default behavior.

Signed-off-by: Derek Buitenhuis 
---
Does this need a avcodec minor/patch version bump?
---
 libavcodec/gif.c | 21 +
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/libavcodec/gif.c b/libavcodec/gif.c
index de41992851..8c07ee2769 100644
--- a/libavcodec/gif.c
+++ b/libavcodec/gif.c
@@ -51,6 +51,7 @@ typedef struct GIFContext {
 AVFrame *last_frame;
 int flags;
 int image;
+int use_global_palette;
 uint32_t palette[AVPALETTE_COUNT];  ///< local reference palette for !pal8
 int palette_loaded;
 int transparent_index;
@@ -293,12 +294,14 @@ static int gif_image_write_image(AVCodecContext *avctx,
 
 bcid = get_palette_transparency_index(global_palette);
 
-bytestream_put_byte(bytestream, 0xf7); /* flags: global clut, 256 
entries */
+bytestream_put_byte(bytestream, ((uint8_t) s->use_global_palette << 7) 
| 0x70 | (s->use_global_palette ? 7 : 0)); /* flags: global clut, 256 entries */
 bytestream_put_byte(bytestream, bcid < 0 ? DEFAULT_TRANSPARENCY_INDEX 
: bcid); /* background color index */
 bytestream_put_byte(bytestream, aspect);
-for (int i = 0; i < 256; i++) {
-const uint32_t v = global_palette[i] & 0xff;
-bytestream_put_be24(bytestream, v);
+if (s->use_global_palette) {
+for (int i = 0; i < 256; i++) {
+const uint32_t v = global_palette[i] & 0xff;
+bytestream_put_be24(bytestream, v);
+}
 }
 }
 
@@ -330,15 +333,16 @@ static int gif_image_write_image(AVCodecContext *avctx,
 bytestream_put_le16(bytestream, width);
 bytestream_put_le16(bytestream, height);
 
-if (!palette) {
-bytestream_put_byte(bytestream, 0x00); /* flags */
-} else {
+if (palette || !s->use_global_palette) {
+const uint32_t *pal = palette ? palette : s->palette;
 unsigned i;
 bytestream_put_byte(bytestream, 1<<7 | 0x7); /* flags */
 for (i = 0; i < AVPALETTE_COUNT; i++) {
-const uint32_t v = palette[i];
+const uint32_t v = pal[i];
 bytestream_put_be24(bytestream, v);
 }
+} else {
+bytestream_put_byte(bytestream, 0x00); /* flags */
 }
 
 bytestream_put_byte(bytestream, 0x08);
@@ -473,6 +477,7 @@ static const AVOption gif_options[] = {
 { "offsetting", "enable picture offsetting", 0, AV_OPT_TYPE_CONST, 
{.i64=GF_OFFSETTING}, INT_MIN, INT_MAX, FLAGS, "flags" },
 { "transdiff", "enable transparency detection between frames", 0, 
AV_OPT_TYPE_CONST, {.i64=GF_TRANSDIFF}, INT_MIN, INT_MAX, FLAGS, "flags" },
 { "gifimage", "enable encoding only images per frame", OFFSET(image), 
AV_OPT_TYPE_BOOL, {.i64=0}, 0, 1, FLAGS },
+{ "global_palette", "write a palette to the global gif header where 
feasible", OFFSET(use_global_palette), AV_OPT_TYPE_BOOL, {.i64=1}, 0, 1, FLAGS 
},
 { NULL }
 };
 
-- 
2.30.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 2/2 v3] avcodec/gifenc: Only write frame palette entries that actually used

[Derek Buitenhuis]

GIF palette entries are not compressed, and writing 256 entries,
which can be up to every frame, uses a significant amount of
space, especially in extreme cases, where palettes can be very
small.

Example, first six seconds of Tears of Steel, palette generated
with libimagequant, 320x240 resolution, and with transparency
optimization + per frame palette:

* Before patch: 186765 bytes
* After patch: 77895 bytes

Signed-off-by: Derek Buitenhuis 
---
Changes since v2:
* shrunk_buf allocated only once per encoder instance.
* Does not use temporary pointers during remapping.
---
 libavcodec/gif.c | 68 +---
 1 file changed, 65 insertions(+), 3 deletions(-)

diff --git a/libavcodec/gif.c b/libavcodec/gif.c
index 8c07ee2769..ff0d50cb44 100644
--- a/libavcodec/gif.c
+++ b/libavcodec/gif.c
@@ -47,6 +47,7 @@ typedef struct GIFContext {
 const AVClass *class;
 LZWState *lzw;
 uint8_t *buf;
+uint8_t *shrunk_buf;
 int buf_size;
 AVFrame *last_frame;
 int flags;
@@ -63,6 +64,38 @@ enum {
 GF_TRANSDIFF  = 1<<1,
 };
 
+static void shrink_palette(const uint32_t *src, uint8_t *map,
+   uint32_t *dst, size_t *palette_count)
+{
+size_t colors_seen = 0;
+
+for (size_t i = 0; i < AVPALETTE_COUNT; i++) {
+int seen = 0;
+for (size_t c = 0; c < colors_seen; c++) {
+if (src[i] == dst[c]) {
+seen = 1;
+break;
+}
+}
+if (!seen) {
+dst[colors_seen] = src[i];
+map[i] = colors_seen;
+colors_seen++;
+}
+}
+
+*palette_count = colors_seen;
+}
+
+static void remap_frame_to_palette(const uint8_t *src, int src_linesize,
+   uint8_t *dst, int dst_linesize,
+   int w, int h, uint8_t *map)
+{
+for (int i = 0; i < h; i++)
+for (int j = 0; j < w; j++)
+dst[i * dst_linesize + j] = map[src[i * src_linesize + j]];
+}
+
 static int is_image_translucent(AVCodecContext *avctx,
 const uint8_t *buf, const int linesize)
 {
@@ -267,6 +300,17 @@ static int gif_image_write_image(AVCodecContext *avctx,
 int x_start = 0, y_start = 0, trans = s->transparent_index;
 int bcid = -1, honor_transparency = (s->flags & GF_TRANSDIFF) && 
s->last_frame && !palette;
 const uint8_t *ptr;
+uint32_t shrunk_palette[AVPALETTE_COUNT];
+uint8_t map[AVPALETTE_COUNT] = { 0 };
+size_t shrunk_palette_count = 0;
+
+/*
+ * We memset to 0xff instead of 0x00 so that the transparency detection
+ * doesn't pick anything after the palette entries as the transparency
+ * index, and because GIF89a requires us to always write a power-of-2
+ * number of palette entries.
+ */
+memset(shrunk_palette, 0xff, AVPALETTE_SIZE);
 
 if (!s->image && is_image_translucent(avctx, buf, linesize)) {
 gif_crop_translucent(avctx, buf, linesize, , , _start, 
_start);
@@ -335,9 +379,14 @@ static int gif_image_write_image(AVCodecContext *avctx,
 
 if (palette || !s->use_global_palette) {
 const uint32_t *pal = palette ? palette : s->palette;
+unsigned pow2_count;
 unsigned i;
-bytestream_put_byte(bytestream, 1<<7 | 0x7); /* flags */
-for (i = 0; i < AVPALETTE_COUNT; i++) {
+
+shrink_palette(pal, map, shrunk_palette, _palette_count);
+pow2_count = av_log2(shrunk_palette_count - 1);
+
+bytestream_put_byte(bytestream, 1<<7 | pow2_count); /* flags */
+for (i = 0; i < 1 << (pow2_count + 1); i++) {
 const uint32_t v = pal[i];
 bytestream_put_be24(bytestream, v);
 }
@@ -350,7 +399,19 @@ static int gif_image_write_image(AVCodecContext *avctx,
 ff_lzw_encode_init(s->lzw, s->buf, s->buf_size,
12, FF_LZW_GIF, 1);
 
-ptr = buf + y_start*linesize + x_start;
+if (shrunk_palette_count) {
+if (!s->shrunk_buf) {
+s->shrunk_buf = av_malloc(avctx->height * linesize);
+if (!s->shrunk_buf) {
+av_log(avctx, AV_LOG_ERROR, "Could not allocated remapped 
frame buffer.\n");
+return AVERROR(ENOMEM);
+}
+}
+remap_frame_to_palette(buf, linesize, s->shrunk_buf, linesize, 
avctx->width, avctx->height, map);
+ptr = s->shrunk_buf + y_start*linesize + x_start;
+} else {
+ptr = buf + y_start*linesize + x_start;
+}
 if (honor_transparency) {
 const int ref_linesize = s->last_frame->linesize[0];
 const uint8_t *ref = s->last_frame->data[0] + y_start*ref_linesize + 
x_start;
@@ -464,6 +525,7 @@ static int gif_encode_close(AVCodecContext *avctx)
 
 av_freep(>lzw);
 av_freep(>buf);
+av_freep(>shrunk_buf);
 s->buf_size = 0;
 av_frame_free(>last_frame);
 av_freep(>tmpl);
-- 
2.30.0

Re: [FFmpeg-devel] [PATCH 2/2 v2] avcodec/gifenc: Only write frame palette entries that actually used

[Andreas Rheinhardt]

Derek Buitenhuis:
> GIF palette entries are not compressed, and writing 256 entries,
> which can be up to every frame, uses a significant amount of
> space, especially in extreme cases, where palettes can be very
> small.
> 
> Example, first six seconds of Tears of Steel, palette generated
> with libimagequant, 320x240 resolution, and with transparency
> optimization + per frame palette:
> 
> * Before patch: 186765 bytes
> * After patch: 77895 bytes
> 
> Signed-off-by: Derek Buitenhuis 
> ---
> The global palette, if used, is no longer ever shrunk in v2 of this
> patch. This is because just because any given frame doesn't reference
> an index, doesn't mean a future frame won't.
> ---
>  libavcodec/gif.c | 72 ++--
>  1 file changed, 69 insertions(+), 3 deletions(-)
> 
> diff --git a/libavcodec/gif.c b/libavcodec/gif.c
> index 8c07ee2769..420a1f523e 100644
> --- a/libavcodec/gif.c
> +++ b/libavcodec/gif.c
> @@ -63,6 +63,44 @@ enum {
>  GF_TRANSDIFF  = 1<<1,
>  };
>  
> +static void shrink_palette(const uint32_t *src, uint8_t *map,
> +   uint32_t *dst, size_t *palette_count)
> +{
> +size_t colors_seen = 0;
> +
> +for (size_t i = 0; i < AVPALETTE_COUNT; i++) {
> +int seen = 0;
> +for (size_t c = 0; c < colors_seen; c++) {
> +if (src[i] == dst[c]) {
> +seen = 1;
> +break;
> +}
> +}
> +if (!seen) {
> +dst[colors_seen] = src[i];
> +map[i] = colors_seen;
> +colors_seen++;
> +}
> +}
> +
> +*palette_count = colors_seen;
> +}
> +
> +static void remap_frame_to_palette(const uint8_t *src, int src_linesize,
> +   uint8_t *dst, int dst_linesize,
> +   int w, int h, uint8_t *map)
> +{
> +uint8_t *sptr = (uint8_t *) src;

Casting const away here is completely unnecessary as sptr is not used to
modify what it points to; and actually sptr and dptr are completely
unnecessary, too.

> +uint8_t *dptr = dst;
> +
> +for (int i = 0; i < h; i++) {
> +for (int j = 0; j < w; j++)
> +dptr[j] = map[sptr[j]];
> +dptr += dst_linesize;
> +sptr += src_linesize;
> +}
> +}
> +
>  static int is_image_translucent(AVCodecContext *avctx,
>  const uint8_t *buf, const int linesize)
>  {
> @@ -267,6 +305,18 @@ static int gif_image_write_image(AVCodecContext *avctx,
>  int x_start = 0, y_start = 0, trans = s->transparent_index;
>  int bcid = -1, honor_transparency = (s->flags & GF_TRANSDIFF) && 
> s->last_frame && !palette;
>  const uint8_t *ptr;
> +uint32_t shrunk_palette[AVPALETTE_COUNT];
> +uint8_t map[AVPALETTE_COUNT] = { 0 };
> +size_t shrunk_palette_count = 0;
> +uint8_t *shrunk_buf = NULL;
> +
> +/*
> + * We memset to 0xff instead of 0x00 so that the transparency detection
> + * doesn't pick anything after the palette entries as the transparency
> + * index, and because GIF89a requires us to always write a power-of-2
> + * number of palette entries.
> + */
> +memset(shrunk_palette, 0xff, AVPALETTE_SIZE);
>  
>  if (!s->image && is_image_translucent(avctx, buf, linesize)) {
>  gif_crop_translucent(avctx, buf, linesize, , , 
> _start, _start);
> @@ -335,9 +385,14 @@ static int gif_image_write_image(AVCodecContext *avctx,
>  
>  if (palette || !s->use_global_palette) {
>  const uint32_t *pal = palette ? palette : s->palette;
> +unsigned pow2_count;
>  unsigned i;
> -bytestream_put_byte(bytestream, 1<<7 | 0x7); /* flags */
> -for (i = 0; i < AVPALETTE_COUNT; i++) {
> +
> +shrink_palette(pal, map, shrunk_palette, _palette_count);
> +pow2_count = av_log2(shrunk_palette_count - 1);
> +
> +bytestream_put_byte(bytestream, 1<<7 | pow2_count); /* flags */
> +for (i = 0; i < 1 << (pow2_count + 1); i++) {
>  const uint32_t v = pal[i];
>  bytestream_put_be24(bytestream, v);
>  }
> @@ -350,7 +405,17 @@ static int gif_image_write_image(AVCodecContext *avctx,
>  ff_lzw_encode_init(s->lzw, s->buf, s->buf_size,
> 12, FF_LZW_GIF, 1);
>  
> -ptr = buf + y_start*linesize + x_start;
> +if (shrunk_palette_count) {
> +shrunk_buf = av_malloc(avctx->height * linesize);
> +if (!shrunk_buf) {
> +av_log(avctx, AV_LOG_ERROR, "Could not allocated remapped frame 
> buffer.\n");
> +return AVERROR(ENOMEM);
> +}
> +remap_frame_to_palette(buf, linesize, shrunk_buf, linesize, 
> avctx->width, avctx->height, map);
> +ptr = shrunk_buf + y_start*linesize + x_start;
> +} else {
> +ptr = buf + y_start*linesize + x_start;
> +}
>  if (honor_transparency) {
>  const int ref_linesize = s->last_frame->linesize[0];
>   

Re: [FFmpeg-devel] [PATCH 2/2 v2] avcodec/gifenc: Only write frame palette entries that actually used

[Paul B Mahol]

On Sat, Feb 20, 2021 at 7:58 PM Derek Buitenhuis 
wrote:

> GIF palette entries are not compressed, and writing 256 entries,
> which can be up to every frame, uses a significant amount of
> space, especially in extreme cases, where palettes can be very
> small.
>
> Example, first six seconds of Tears of Steel, palette generated
> with libimagequant, 320x240 resolution, and with transparency
> optimization + per frame palette:
>
> * Before patch: 186765 bytes
> * After patch: 77895 bytes
>
> Signed-off-by: Derek Buitenhuis 
> ---
> The global palette, if used, is no longer ever shrunk in v2 of this
> patch. This is because just because any given frame doesn't reference
> an index, doesn't mean a future frame won't.
> ---
>  libavcodec/gif.c | 72 ++--
>  1 file changed, 69 insertions(+), 3 deletions(-)
>
> diff --git a/libavcodec/gif.c b/libavcodec/gif.c
> index 8c07ee2769..420a1f523e 100644
> --- a/libavcodec/gif.c
> +++ b/libavcodec/gif.c
> @@ -63,6 +63,44 @@ enum {
>  GF_TRANSDIFF  = 1<<1,
>  };
>
> +static void shrink_palette(const uint32_t *src, uint8_t *map,
> +   uint32_t *dst, size_t *palette_count)
> +{
> +size_t colors_seen = 0;
> +
> +for (size_t i = 0; i < AVPALETTE_COUNT; i++) {
> +int seen = 0;
> +for (size_t c = 0; c < colors_seen; c++) {
> +if (src[i] == dst[c]) {
> +seen = 1;
> +break;
> +}
> +}
> +if (!seen) {
> +dst[colors_seen] = src[i];
> +map[i] = colors_seen;
> +colors_seen++;
> +}
> +}
> +
> +*palette_count = colors_seen;
> +}
> +
> +static void remap_frame_to_palette(const uint8_t *src, int src_linesize,
> +   uint8_t *dst, int dst_linesize,
> +   int w, int h, uint8_t *map)
> +{
> +uint8_t *sptr = (uint8_t *) src;
> +uint8_t *dptr = dst;
> +
> +for (int i = 0; i < h; i++) {
> +for (int j = 0; j < w; j++)
> +dptr[j] = map[sptr[j]];
> +dptr += dst_linesize;
> +sptr += src_linesize;
> +}
> +}
> +
>  static int is_image_translucent(AVCodecContext *avctx,
>  const uint8_t *buf, const int linesize)
>  {
> @@ -267,6 +305,18 @@ static int gif_image_write_image(AVCodecContext
> *avctx,
>  int x_start = 0, y_start = 0, trans = s->transparent_index;
>  int bcid = -1, honor_transparency = (s->flags & GF_TRANSDIFF) &&
> s->last_frame && !palette;
>  const uint8_t *ptr;
> +uint32_t shrunk_palette[AVPALETTE_COUNT];
> +uint8_t map[AVPALETTE_COUNT] = { 0 };
> +size_t shrunk_palette_count = 0;
> +uint8_t *shrunk_buf = NULL;
> +
> +/*
> + * We memset to 0xff instead of 0x00 so that the transparency
> detection
> + * doesn't pick anything after the palette entries as the transparency
> + * index, and because GIF89a requires us to always write a power-of-2
> + * number of palette entries.
> + */
> +memset(shrunk_palette, 0xff, AVPALETTE_SIZE);
>
>  if (!s->image && is_image_translucent(avctx, buf, linesize)) {
>  gif_crop_translucent(avctx, buf, linesize, , ,
> _start, _start);
> @@ -335,9 +385,14 @@ static int gif_image_write_image(AVCodecContext
> *avctx,
>
>  if (palette || !s->use_global_palette) {
>  const uint32_t *pal = palette ? palette : s->palette;
> +unsigned pow2_count;
>  unsigned i;
> -bytestream_put_byte(bytestream, 1<<7 | 0x7); /* flags */
> -for (i = 0; i < AVPALETTE_COUNT; i++) {
> +
> +shrink_palette(pal, map, shrunk_palette, _palette_count);
> +pow2_count = av_log2(shrunk_palette_count - 1);
> +
> +bytestream_put_byte(bytestream, 1<<7 | pow2_count); /* flags */
> +for (i = 0; i < 1 << (pow2_count + 1); i++) {
>  const uint32_t v = pal[i];
>  bytestream_put_be24(bytestream, v);
>  }
> @@ -350,7 +405,17 @@ static int gif_image_write_image(AVCodecContext
> *avctx,
>  ff_lzw_encode_init(s->lzw, s->buf, s->buf_size,
> 12, FF_LZW_GIF, 1);
>
> -ptr = buf + y_start*linesize + x_start;
> +if (shrunk_palette_count) {
> +shrunk_buf = av_malloc(avctx->height * linesize);
>

I would prefer it this allocation is done only once per encoder instance.


> +if (!shrunk_buf) {
> +av_log(avctx, AV_LOG_ERROR, "Could not allocated remapped
> frame buffer.\n");
> +return AVERROR(ENOMEM);
> +}
> +remap_frame_to_palette(buf, linesize, shrunk_buf, linesize,
> avctx->width, avctx->height, map);
> +ptr = shrunk_buf + y_start*linesize + x_start;
> +} else {
> +ptr = buf + y_start*linesize + x_start;
> +}
>  if (honor_transparency) {
>  const int ref_linesize = s->last_frame->linesize[0];
>  const uint8_t *ref = s->last_frame->data[0] +

[FFmpeg-devel] [PATCH 2/2 v2] avcodec/gifenc: Only write frame palette entries that actually used

[Derek Buitenhuis]

GIF palette entries are not compressed, and writing 256 entries,
which can be up to every frame, uses a significant amount of
space, especially in extreme cases, where palettes can be very
small.

Example, first six seconds of Tears of Steel, palette generated
with libimagequant, 320x240 resolution, and with transparency
optimization + per frame palette:

* Before patch: 186765 bytes
* After patch: 77895 bytes

Signed-off-by: Derek Buitenhuis 
---
The global palette, if used, is no longer ever shrunk in v2 of this
patch. This is because just because any given frame doesn't reference
an index, doesn't mean a future frame won't.
---
 libavcodec/gif.c | 72 ++--
 1 file changed, 69 insertions(+), 3 deletions(-)

diff --git a/libavcodec/gif.c b/libavcodec/gif.c
index 8c07ee2769..420a1f523e 100644
--- a/libavcodec/gif.c
+++ b/libavcodec/gif.c
@@ -63,6 +63,44 @@ enum {
 GF_TRANSDIFF  = 1<<1,
 };
 
+static void shrink_palette(const uint32_t *src, uint8_t *map,
+   uint32_t *dst, size_t *palette_count)
+{
+size_t colors_seen = 0;
+
+for (size_t i = 0; i < AVPALETTE_COUNT; i++) {
+int seen = 0;
+for (size_t c = 0; c < colors_seen; c++) {
+if (src[i] == dst[c]) {
+seen = 1;
+break;
+}
+}
+if (!seen) {
+dst[colors_seen] = src[i];
+map[i] = colors_seen;
+colors_seen++;
+}
+}
+
+*palette_count = colors_seen;
+}
+
+static void remap_frame_to_palette(const uint8_t *src, int src_linesize,
+   uint8_t *dst, int dst_linesize,
+   int w, int h, uint8_t *map)
+{
+uint8_t *sptr = (uint8_t *) src;
+uint8_t *dptr = dst;
+
+for (int i = 0; i < h; i++) {
+for (int j = 0; j < w; j++)
+dptr[j] = map[sptr[j]];
+dptr += dst_linesize;
+sptr += src_linesize;
+}
+}
+
 static int is_image_translucent(AVCodecContext *avctx,
 const uint8_t *buf, const int linesize)
 {
@@ -267,6 +305,18 @@ static int gif_image_write_image(AVCodecContext *avctx,
 int x_start = 0, y_start = 0, trans = s->transparent_index;
 int bcid = -1, honor_transparency = (s->flags & GF_TRANSDIFF) && 
s->last_frame && !palette;
 const uint8_t *ptr;
+uint32_t shrunk_palette[AVPALETTE_COUNT];
+uint8_t map[AVPALETTE_COUNT] = { 0 };
+size_t shrunk_palette_count = 0;
+uint8_t *shrunk_buf = NULL;
+
+/*
+ * We memset to 0xff instead of 0x00 so that the transparency detection
+ * doesn't pick anything after the palette entries as the transparency
+ * index, and because GIF89a requires us to always write a power-of-2
+ * number of palette entries.
+ */
+memset(shrunk_palette, 0xff, AVPALETTE_SIZE);
 
 if (!s->image && is_image_translucent(avctx, buf, linesize)) {
 gif_crop_translucent(avctx, buf, linesize, , , _start, 
_start);
@@ -335,9 +385,14 @@ static int gif_image_write_image(AVCodecContext *avctx,
 
 if (palette || !s->use_global_palette) {
 const uint32_t *pal = palette ? palette : s->palette;
+unsigned pow2_count;
 unsigned i;
-bytestream_put_byte(bytestream, 1<<7 | 0x7); /* flags */
-for (i = 0; i < AVPALETTE_COUNT; i++) {
+
+shrink_palette(pal, map, shrunk_palette, _palette_count);
+pow2_count = av_log2(shrunk_palette_count - 1);
+
+bytestream_put_byte(bytestream, 1<<7 | pow2_count); /* flags */
+for (i = 0; i < 1 << (pow2_count + 1); i++) {
 const uint32_t v = pal[i];
 bytestream_put_be24(bytestream, v);
 }
@@ -350,7 +405,17 @@ static int gif_image_write_image(AVCodecContext *avctx,
 ff_lzw_encode_init(s->lzw, s->buf, s->buf_size,
12, FF_LZW_GIF, 1);
 
-ptr = buf + y_start*linesize + x_start;
+if (shrunk_palette_count) {
+shrunk_buf = av_malloc(avctx->height * linesize);
+if (!shrunk_buf) {
+av_log(avctx, AV_LOG_ERROR, "Could not allocated remapped frame 
buffer.\n");
+return AVERROR(ENOMEM);
+}
+remap_frame_to_palette(buf, linesize, shrunk_buf, linesize, 
avctx->width, avctx->height, map);
+ptr = shrunk_buf + y_start*linesize + x_start;
+} else {
+ptr = buf + y_start*linesize + x_start;
+}
 if (honor_transparency) {
 const int ref_linesize = s->last_frame->linesize[0];
 const uint8_t *ref = s->last_frame->data[0] + y_start*ref_linesize + 
x_start;
@@ -383,6 +448,7 @@ static int gif_image_write_image(AVCodecContext *avctx,
 len -= size;
 }
 bytestream_put_byte(bytestream, 0x00); /* end of image block */
+av_freep(_buf);
 return 0;
 }
 
-- 
2.30.0

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org

[FFmpeg-devel] [PATCH 0/2 v2] GIF Palette Improvements

[Derek Buitenhuis]

Derek Buitenhuis (2):
  avcodec/gifenc: Add global_palette option
  avcodec/gifenc: Only write frame palette entries that actually used

 libavcodec/gif.c | 93 ++--
 1 file changed, 82 insertions(+), 11 deletions(-)

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

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/dpxenc: write framerate to header

[Paul B Mahol]

On Sat, Feb 20, 2021 at 7:02 PM Andreas Rheinhardt <
andreas.rheinha...@gmail.com> wrote:

> James Almer:
> > On 2/20/2021 1:37 PM, Paul B Mahol wrote:
> >> Signed-off-by: Paul B Mahol 
> >> ---
> >>   libavcodec/dpxenc.c   | 21 ++---
> >>   tests/ref/lavf/gbrp10le.dpx   |  4 ++--
> >>   tests/ref/lavf/gbrp12le.dpx   |  4 ++--
> >>   tests/ref/lavf/rgb48le.dpx|  4 ++--
> >>   tests/ref/lavf/rgb48le_10.dpx |  4 ++--
> >>   tests/ref/lavf/rgba64le.dpx   |  4 ++--
> >>   6 files changed, 28 insertions(+), 13 deletions(-)
> >>
> >> diff --git a/libavcodec/dpxenc.c b/libavcodec/dpxenc.c
> >> index a5960334d5..b296f9f22e 100644
> >> --- a/libavcodec/dpxenc.c
> >> +++ b/libavcodec/dpxenc.c
> >> @@ -173,14 +173,25 @@ static void encode_gbrp12(AVCodecContext *avctx,
> >> const AVFrame *pic, uint16_t *d
> >>   }
> >>   }
> >>   +#define FILE_HEADER_SIZE 768
> >> +#define IMAGE_HEADER_SIZE 640
> >> +#define ORIENTATION_HEADER_SIZE 256
> >> +#define FILM_INFO_HEADER_SIZE 256
> >> +#define TV_INFO_HEADER_SIZE 128
> >> +#define HEADER_SIZE (FILE_HEADER_SIZE + \
> >> + IMAGE_HEADER_SIZE + \
> >> + ORIENTATION_HEADER_SIZE + \
> >> + FILM_INFO_HEADER_SIZE + \
> >> + TV_INFO_HEADER_SIZE)
> >> +
> >>   static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
> >>   const AVFrame *frame, int *got_packet)
> >>   {
> >>   DPXContext *s = avctx->priv_data;
> >> -int size, ret, need_align, len;
> >> +int ret, need_align, len;
> >> +int64_t size;
> >>   uint8_t *buf;
> >>   -#define HEADER_SIZE 1664  /* DPX Generic header */
> >>   if (s->bits_per_component == 10)
> >>   size = avctx->height * avctx->width * 4;
> >>   else if (s->bits_per_component == 12) {
> >> @@ -196,7 +207,7 @@ static int encode_frame(AVCodecContext *avctx,
> >> AVPacket *pkt,
> >>   need_align = size - len;
> >>   size *= avctx->height;
> >>   }
> >> -if ((ret = ff_alloc_packet2(avctx, pkt, size + HEADER_SIZE, 0)) <
> 0)
> >> +if ((ret = ff_alloc_packet2(avctx, pkt, HEADER_SIZE + size, 0)) <
> 0)
> >
> > This looks like pointless change. But you could while at it set min_size
> > to the same value as size.
> >
> >>   return ret;
> >>   buf = pkt->data;
> >>   @@ -229,6 +240,10 @@ static int encode_frame(AVCodecContext *avctx,
> >> AVPacket *pkt,
> >>   write32(buf + 1628, avctx->sample_aspect_ratio.num);
> >>   write32(buf + 1632, avctx->sample_aspect_ratio.den);
> >>   +/* Film information header */
> >> +if (avctx->framerate.num && avctx->framerate.den)
> >> +write32(buf + 1724, av_float2int(av_q2d(avctx->framerate)));
> >> +
> >>   switch(s->bits_per_component) {
> >>   case 8:
> >>   case 16:
> >> diff --git a/tests/ref/lavf/gbrp10le.dpx b/tests/ref/lavf/gbrp10le.dpx
> >> index b33da34e20..7c03dc0779 100644
> >> --- a/tests/ref/lavf/gbrp10le.dpx
> >> +++ b/tests/ref/lavf/gbrp10le.dpx
> >> @@ -1,3 +1,3 @@
> >> -7ca935d5d5e00c54acbc85565d3039b6
> >> *tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
> >> +c7c8ecd9d8c8a2c4dce6d92bfb9877d7
> >> *tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
> >>   tests/data/images/gbrp10le.dpx/%02d.gbrp10le.dpx CRC=0xe6663fba
> >> -407168 tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
> >> +407552 tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
> >
> > Nearly 400 more bytes to write a 4 byte long frame rate field?
> >
>
> That's because HEADER_SIZE has been increased from 1664 to 2048. Don't
> know if it's necessary.
>

Please consult specification of DPX, you will find all there.


>
> >> diff --git a/tests/ref/lavf/gbrp12le.dpx b/tests/ref/lavf/gbrp12le.dpx
> >> index e2e794ecc6..5ca855b004 100644
> >> --- a/tests/ref/lavf/gbrp12le.dpx
> >> +++ b/tests/ref/lavf/gbrp12le.dpx
> >> @@ -1,3 +1,3 @@
> >> -a4cfea1797c928f2eff73573e559675d
> >> *tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
> >> +79dcf3b32ed8e627a68bba19bf625ca5
> >> *tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
> >>   tests/data/images/gbrp12le.dpx/%02d.gbrp12le.dpx CRC=0x1c755633
> >> -609920 tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
> >> +610304 tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
> >> diff --git a/tests/ref/lavf/rgb48le.dpx b/tests/ref/lavf/rgb48le.dpx
> >> index 073153898a..33817d95a9 100644
> >> --- a/tests/ref/lavf/rgb48le.dpx
> >> +++ b/tests/ref/lavf/rgb48le.dpx
> >> @@ -1,3 +1,3 @@
> >> -075963c3c08978b6a20555ba09161434
> >> *tests/data/images/rgb48le.dpx/02.rgb48le.dpx
> >> +51c703863c9df1db5ef78a77dd5fbd0f
> >> *tests/data/images/rgb48le.dpx/02.rgb48le.dpx
> >>   tests/data/images/rgb48le.dpx/%02d.rgb48le.dpx CRC=0xe5b9c023
> >> -609920 tests/data/images/rgb48le.dpx/02.rgb48le.dpx
> >> +610304 tests/data/images/rgb48le.dpx/02.rgb48le.dpx
> >> diff --git a/tests/ref/lavf/rgb48le_10.dpx
> >> b/tests/ref/lavf/rgb48le_10.dpx
> >> index ce36e5079f..3c934168b7 100644
> >> --- 

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/dpxenc: write framerate to header

[Andreas Rheinhardt]

James Almer:
> On 2/20/2021 1:37 PM, Paul B Mahol wrote:
>> Signed-off-by: Paul B Mahol 
>> ---
>>   libavcodec/dpxenc.c   | 21 ++---
>>   tests/ref/lavf/gbrp10le.dpx   |  4 ++--
>>   tests/ref/lavf/gbrp12le.dpx   |  4 ++--
>>   tests/ref/lavf/rgb48le.dpx    |  4 ++--
>>   tests/ref/lavf/rgb48le_10.dpx |  4 ++--
>>   tests/ref/lavf/rgba64le.dpx   |  4 ++--
>>   6 files changed, 28 insertions(+), 13 deletions(-)
>>
>> diff --git a/libavcodec/dpxenc.c b/libavcodec/dpxenc.c
>> index a5960334d5..b296f9f22e 100644
>> --- a/libavcodec/dpxenc.c
>> +++ b/libavcodec/dpxenc.c
>> @@ -173,14 +173,25 @@ static void encode_gbrp12(AVCodecContext *avctx,
>> const AVFrame *pic, uint16_t *d
>>   }
>>   }
>>   +#define FILE_HEADER_SIZE 768
>> +#define IMAGE_HEADER_SIZE 640
>> +#define ORIENTATION_HEADER_SIZE 256
>> +#define FILM_INFO_HEADER_SIZE 256
>> +#define TV_INFO_HEADER_SIZE 128
>> +#define HEADER_SIZE (FILE_HEADER_SIZE + \
>> + IMAGE_HEADER_SIZE + \
>> + ORIENTATION_HEADER_SIZE + \
>> + FILM_INFO_HEADER_SIZE + \
>> + TV_INFO_HEADER_SIZE)
>> +
>>   static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
>>   const AVFrame *frame, int *got_packet)
>>   {
>>   DPXContext *s = avctx->priv_data;
>> -    int size, ret, need_align, len;
>> +    int ret, need_align, len;
>> +    int64_t size;
>>   uint8_t *buf;
>>   -#define HEADER_SIZE 1664  /* DPX Generic header */
>>   if (s->bits_per_component == 10)
>>   size = avctx->height * avctx->width * 4;
>>   else if (s->bits_per_component == 12) {
>> @@ -196,7 +207,7 @@ static int encode_frame(AVCodecContext *avctx,
>> AVPacket *pkt,
>>   need_align = size - len;
>>   size *= avctx->height;
>>   }
>> -    if ((ret = ff_alloc_packet2(avctx, pkt, size + HEADER_SIZE, 0)) < 0)
>> +    if ((ret = ff_alloc_packet2(avctx, pkt, HEADER_SIZE + size, 0)) < 0)
> 
> This looks like pointless change. But you could while at it set min_size
> to the same value as size.
> 
>>   return ret;
>>   buf = pkt->data;
>>   @@ -229,6 +240,10 @@ static int encode_frame(AVCodecContext *avctx,
>> AVPacket *pkt,
>>   write32(buf + 1628, avctx->sample_aspect_ratio.num);
>>   write32(buf + 1632, avctx->sample_aspect_ratio.den);
>>   +    /* Film information header */
>> +    if (avctx->framerate.num && avctx->framerate.den)
>> +    write32(buf + 1724, av_float2int(av_q2d(avctx->framerate)));
>> +
>>   switch(s->bits_per_component) {
>>   case 8:
>>   case 16:
>> diff --git a/tests/ref/lavf/gbrp10le.dpx b/tests/ref/lavf/gbrp10le.dpx
>> index b33da34e20..7c03dc0779 100644
>> --- a/tests/ref/lavf/gbrp10le.dpx
>> +++ b/tests/ref/lavf/gbrp10le.dpx
>> @@ -1,3 +1,3 @@
>> -7ca935d5d5e00c54acbc85565d3039b6
>> *tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
>> +c7c8ecd9d8c8a2c4dce6d92bfb9877d7
>> *tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
>>   tests/data/images/gbrp10le.dpx/%02d.gbrp10le.dpx CRC=0xe6663fba
>> -407168 tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
>> +407552 tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
> 
> Nearly 400 more bytes to write a 4 byte long frame rate field?
> 

That's because HEADER_SIZE has been increased from 1664 to 2048. Don't
know if it's necessary.

>> diff --git a/tests/ref/lavf/gbrp12le.dpx b/tests/ref/lavf/gbrp12le.dpx
>> index e2e794ecc6..5ca855b004 100644
>> --- a/tests/ref/lavf/gbrp12le.dpx
>> +++ b/tests/ref/lavf/gbrp12le.dpx
>> @@ -1,3 +1,3 @@
>> -a4cfea1797c928f2eff73573e559675d
>> *tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
>> +79dcf3b32ed8e627a68bba19bf625ca5
>> *tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
>>   tests/data/images/gbrp12le.dpx/%02d.gbrp12le.dpx CRC=0x1c755633
>> -609920 tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
>> +610304 tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
>> diff --git a/tests/ref/lavf/rgb48le.dpx b/tests/ref/lavf/rgb48le.dpx
>> index 073153898a..33817d95a9 100644
>> --- a/tests/ref/lavf/rgb48le.dpx
>> +++ b/tests/ref/lavf/rgb48le.dpx
>> @@ -1,3 +1,3 @@
>> -075963c3c08978b6a20555ba09161434
>> *tests/data/images/rgb48le.dpx/02.rgb48le.dpx
>> +51c703863c9df1db5ef78a77dd5fbd0f
>> *tests/data/images/rgb48le.dpx/02.rgb48le.dpx
>>   tests/data/images/rgb48le.dpx/%02d.rgb48le.dpx CRC=0xe5b9c023
>> -609920 tests/data/images/rgb48le.dpx/02.rgb48le.dpx
>> +610304 tests/data/images/rgb48le.dpx/02.rgb48le.dpx
>> diff --git a/tests/ref/lavf/rgb48le_10.dpx
>> b/tests/ref/lavf/rgb48le_10.dpx
>> index ce36e5079f..3c934168b7 100644
>> --- a/tests/ref/lavf/rgb48le_10.dpx
>> +++ b/tests/ref/lavf/rgb48le_10.dpx
>> @@ -1,3 +1,3 @@
>> -b9f22728f8ff393bf30cf6cbd624fa95
>> *tests/data/images/rgb48le_10.dpx/02.rgb48le_10.dpx
>> +a0ca7132c33a5c0eb25dd4a2b6117743
>> *tests/data/images/rgb48le_10.dpx/02.rgb48le_10.dpx
>>   tests/data/images/rgb48le_10.dpx/%02d.rgb48le_10.dpx CRC=0xf38d5830
>> -407168 

Re: [FFmpeg-devel] [PATCH] avformat/dv: fix timestamps of audio packets in case of dropped corrupt audio frames

[Dave Rice]

Hi,

> On Oct 31, 2020, at 5:15 PM, Marton Balint  > wrote:
> On Sat, 31 Oct 2020, Dave Rice wrote:
>>> On Oct 31, 2020, at 3:47 PM, Marton Balint >> > wrote:
>>> On Sat, 31 Oct 2020, Dave Rice wrote:
 Hi Marton,
> On Oct 31, 2020, at 12:56 PM, Marton Balint  > wrote:
> Fixes out of sync timestamps in ticket #8762.
 Although Michael’s recent patch does address the issue documented in 8762, 
 I haven’t found this patch to fix the issue. I tried with -c:a copy and 
 with -c:a pcm_s16le with some sample files that exhibit this issue but 
 each output was out of sync. I put an output at 
 https://gist.github.com/dericed/659bd843bd38b6f24a60198b5e345795 
 . That 
 output notes that 3597 packages of video are read and 3586 packets of 
 audio. In the resulting file, at the end of the timeline the audio is 9 
 frames out of sync and my output video stream is 00:02:00.020 and output 
 audio stream is 00:01:59.653.
 Beyond copying or encoding the audio, are there other options I should use 
 to test this?
>>> Well, it depends on what you want. After this patch you should get a file 
>>> which has audio packets synced to video, but the audio stream is sparse, 
>>> not every video packet has a corresponding audio packet. (It looks like our 
>>> MOV muxer does not support muxing of sparse audio therefore does not 
>>> produce proper timestamps. But MKV does, please try that.)
>>> You can also make ffmpeg generate the missing audio based on packet 
>>> timestamps. Swresample has an async=1 option, so something like this should 
>>> get you synced audio with continous audio packets:
>>> ffmpeg -y -i 167052_12.dv -c:v copy \
>>> -af aresample=async=1:min_hard_comp=0.01 -c:a pcm_s16le 167052_12.mov
>> 
>> Thank you for this. With the patch and async, the result is synced and the 
>> resulting audio was the same as Michael’s patch.
>> 
>> Could you explain why you used min_hard_comp here? IIUC min_hard_comp is a 
>> set a threshold between the strategies of trim/fill or stretch/squeeze to 
>> align the audio to time; however, the async documentation says "Setting this 
>> to 1 will enable filling and trimming, larger values represent the maximum 
>> amount in samples that the data may be stretched or squeezed” so I thought 
>> that async=1 would not permit stretch/squeeze anyway.
> 
> It is documented poorly, but if you check the source code you will see that 
> async=1 implicitly sets min_comp to 0.001 enabling trimming/dropping. 
> min_hard_comp decides the threshold when silence injection actually happens, 
> and the default for that is 0.1, which is more than a frame, therefore not 
> acceptable if we want to maintain <1 frame accuracy. Or at least that is how 
> I think it should work.

I’ve found that aresample=async=1:min_hard_comp=0.01, as discussed here, works 
well to add audio samples to maintain timestamp accuracy when muxing into a 
format like mov. However, this approach doesn’t work if the sparseness of the 
audio stream is at the end of the stream. Is there a way to use min_hard_comp 
to consider differences between a timestamp and audio data when one of the ends 
of that range is the end of the file?
Best Regards,
Dave Rice

___
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/dpxenc: write framerate to header

[Paul B Mahol]

On Sat, Feb 20, 2021 at 6:17 PM James Almer  wrote:

> On 2/20/2021 1:37 PM, Paul B Mahol wrote:
> > Signed-off-by: Paul B Mahol 
> > ---
> >   libavcodec/dpxenc.c   | 21 ++---
> >   tests/ref/lavf/gbrp10le.dpx   |  4 ++--
> >   tests/ref/lavf/gbrp12le.dpx   |  4 ++--
> >   tests/ref/lavf/rgb48le.dpx|  4 ++--
> >   tests/ref/lavf/rgb48le_10.dpx |  4 ++--
> >   tests/ref/lavf/rgba64le.dpx   |  4 ++--
> >   6 files changed, 28 insertions(+), 13 deletions(-)
> >
> > diff --git a/libavcodec/dpxenc.c b/libavcodec/dpxenc.c
> > index a5960334d5..b296f9f22e 100644
> > --- a/libavcodec/dpxenc.c
> > +++ b/libavcodec/dpxenc.c
> > @@ -173,14 +173,25 @@ static void encode_gbrp12(AVCodecContext *avctx,
> const AVFrame *pic, uint16_t *d
> >   }
> >   }
> >
> > +#define FILE_HEADER_SIZE 768
> > +#define IMAGE_HEADER_SIZE 640
> > +#define ORIENTATION_HEADER_SIZE 256
> > +#define FILM_INFO_HEADER_SIZE 256
> > +#define TV_INFO_HEADER_SIZE 128
> > +#define HEADER_SIZE (FILE_HEADER_SIZE + \
> > + IMAGE_HEADER_SIZE + \
> > + ORIENTATION_HEADER_SIZE + \
> > + FILM_INFO_HEADER_SIZE + \
> > + TV_INFO_HEADER_SIZE)
> > +
> >   static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
> >   const AVFrame *frame, int *got_packet)
> >   {
> >   DPXContext *s = avctx->priv_data;
> > -int size, ret, need_align, len;
> > +int ret, need_align, len;
> > +int64_t size;
> >   uint8_t *buf;
> >
> > -#define HEADER_SIZE 1664  /* DPX Generic header */
> >   if (s->bits_per_component == 10)
> >   size = avctx->height * avctx->width * 4;
> >   else if (s->bits_per_component == 12) {
> > @@ -196,7 +207,7 @@ static int encode_frame(AVCodecContext *avctx,
> AVPacket *pkt,
> >   need_align = size - len;
> >   size *= avctx->height;
> >   }
> > -if ((ret = ff_alloc_packet2(avctx, pkt, size + HEADER_SIZE, 0)) < 0)
> > +if ((ret = ff_alloc_packet2(avctx, pkt, HEADER_SIZE + size, 0)) < 0)
>
> This looks like pointless change. But you could while at it set min_size
> to the same value as size.
>
> >   return ret;
> >   buf = pkt->data;
> >
> > @@ -229,6 +240,10 @@ static int encode_frame(AVCodecContext *avctx,
> AVPacket *pkt,
> >   write32(buf + 1628, avctx->sample_aspect_ratio.num);
> >   write32(buf + 1632, avctx->sample_aspect_ratio.den);
> >
> > +/* Film information header */
> > +if (avctx->framerate.num && avctx->framerate.den)
> > +write32(buf + 1724, av_float2int(av_q2d(avctx->framerate)));
> > +
> >   switch(s->bits_per_component) {
> >   case 8:
> >   case 16:
> > diff --git a/tests/ref/lavf/gbrp10le.dpx b/tests/ref/lavf/gbrp10le.dpx
> > index b33da34e20..7c03dc0779 100644
> > --- a/tests/ref/lavf/gbrp10le.dpx
> > +++ b/tests/ref/lavf/gbrp10le.dpx
> > @@ -1,3 +1,3 @@
> > -7ca935d5d5e00c54acbc85565d3039b6
> *tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
> > +c7c8ecd9d8c8a2c4dce6d92bfb9877d7
> *tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
> >   tests/data/images/gbrp10le.dpx/%02d.gbrp10le.dpx CRC=0xe6663fba
> > -407168 tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
> > +407552 tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
>
> Nearly 400 more bytes to write a 4 byte long frame rate field?
>

This actually follows specification.

>
> > diff --git a/tests/ref/lavf/gbrp12le.dpx b/tests/ref/lavf/gbrp12le.dpx
> > index e2e794ecc6..5ca855b004 100644
> > --- a/tests/ref/lavf/gbrp12le.dpx
> > +++ b/tests/ref/lavf/gbrp12le.dpx
> > @@ -1,3 +1,3 @@
> > -a4cfea1797c928f2eff73573e559675d
> *tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
> > +79dcf3b32ed8e627a68bba19bf625ca5
> *tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
> >   tests/data/images/gbrp12le.dpx/%02d.gbrp12le.dpx CRC=0x1c755633
> > -609920 tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
> > +610304 tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
> > diff --git a/tests/ref/lavf/rgb48le.dpx b/tests/ref/lavf/rgb48le.dpx
> > index 073153898a..33817d95a9 100644
> > --- a/tests/ref/lavf/rgb48le.dpx
> > +++ b/tests/ref/lavf/rgb48le.dpx
> > @@ -1,3 +1,3 @@
> > -075963c3c08978b6a20555ba09161434
> *tests/data/images/rgb48le.dpx/02.rgb48le.dpx
> > +51c703863c9df1db5ef78a77dd5fbd0f
> *tests/data/images/rgb48le.dpx/02.rgb48le.dpx
> >   tests/data/images/rgb48le.dpx/%02d.rgb48le.dpx CRC=0xe5b9c023
> > -609920 tests/data/images/rgb48le.dpx/02.rgb48le.dpx
> > +610304 tests/data/images/rgb48le.dpx/02.rgb48le.dpx
> > diff --git a/tests/ref/lavf/rgb48le_10.dpx
> b/tests/ref/lavf/rgb48le_10.dpx
> > index ce36e5079f..3c934168b7 100644
> > --- a/tests/ref/lavf/rgb48le_10.dpx
> > +++ b/tests/ref/lavf/rgb48le_10.dpx
> > @@ -1,3 +1,3 @@
> > -b9f22728f8ff393bf30cf6cbd624fa95
> *tests/data/images/rgb48le_10.dpx/02.rgb48le_10.dpx
> > +a0ca7132c33a5c0eb25dd4a2b6117743
> *tests/data/images/rgb48le_10.dpx/02.rgb48le_10.dpx
> >   

Re: [FFmpeg-devel] [PATCH 1/2] avcodec/dpxenc: write framerate to header

[James Almer]

On 2/20/2021 1:37 PM, Paul B Mahol wrote:

Signed-off-by: Paul B Mahol 
---
  libavcodec/dpxenc.c   | 21 ++---
  tests/ref/lavf/gbrp10le.dpx   |  4 ++--
  tests/ref/lavf/gbrp12le.dpx   |  4 ++--
  tests/ref/lavf/rgb48le.dpx|  4 ++--
  tests/ref/lavf/rgb48le_10.dpx |  4 ++--
  tests/ref/lavf/rgba64le.dpx   |  4 ++--
  6 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/libavcodec/dpxenc.c b/libavcodec/dpxenc.c
index a5960334d5..b296f9f22e 100644
--- a/libavcodec/dpxenc.c
+++ b/libavcodec/dpxenc.c
@@ -173,14 +173,25 @@ static void encode_gbrp12(AVCodecContext *avctx, const 
AVFrame *pic, uint16_t *d
  }
  }
  
+#define FILE_HEADER_SIZE 768

+#define IMAGE_HEADER_SIZE 640
+#define ORIENTATION_HEADER_SIZE 256
+#define FILM_INFO_HEADER_SIZE 256
+#define TV_INFO_HEADER_SIZE 128
+#define HEADER_SIZE (FILE_HEADER_SIZE + \
+ IMAGE_HEADER_SIZE + \
+ ORIENTATION_HEADER_SIZE + \
+ FILM_INFO_HEADER_SIZE + \
+ TV_INFO_HEADER_SIZE)
+
  static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
  const AVFrame *frame, int *got_packet)
  {
  DPXContext *s = avctx->priv_data;
-int size, ret, need_align, len;
+int ret, need_align, len;
+int64_t size;
  uint8_t *buf;
  
-#define HEADER_SIZE 1664  /* DPX Generic header */

  if (s->bits_per_component == 10)
  size = avctx->height * avctx->width * 4;
  else if (s->bits_per_component == 12) {
@@ -196,7 +207,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket 
*pkt,
  need_align = size - len;
  size *= avctx->height;
  }
-if ((ret = ff_alloc_packet2(avctx, pkt, size + HEADER_SIZE, 0)) < 0)
+if ((ret = ff_alloc_packet2(avctx, pkt, HEADER_SIZE + size, 0)) < 0)


This looks like pointless change. But you could while at it set min_size 
to the same value as size.



  return ret;
  buf = pkt->data;
  
@@ -229,6 +240,10 @@ static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,

  write32(buf + 1628, avctx->sample_aspect_ratio.num);
  write32(buf + 1632, avctx->sample_aspect_ratio.den);
  
+/* Film information header */

+if (avctx->framerate.num && avctx->framerate.den)
+write32(buf + 1724, av_float2int(av_q2d(avctx->framerate)));
+
  switch(s->bits_per_component) {
  case 8:
  case 16:
diff --git a/tests/ref/lavf/gbrp10le.dpx b/tests/ref/lavf/gbrp10le.dpx
index b33da34e20..7c03dc0779 100644
--- a/tests/ref/lavf/gbrp10le.dpx
+++ b/tests/ref/lavf/gbrp10le.dpx
@@ -1,3 +1,3 @@
-7ca935d5d5e00c54acbc85565d3039b6 
*tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
+c7c8ecd9d8c8a2c4dce6d92bfb9877d7 
*tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
  tests/data/images/gbrp10le.dpx/%02d.gbrp10le.dpx CRC=0xe6663fba
-407168 tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
+407552 tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx


Nearly 400 more bytes to write a 4 byte long frame rate field?


diff --git a/tests/ref/lavf/gbrp12le.dpx b/tests/ref/lavf/gbrp12le.dpx
index e2e794ecc6..5ca855b004 100644
--- a/tests/ref/lavf/gbrp12le.dpx
+++ b/tests/ref/lavf/gbrp12le.dpx
@@ -1,3 +1,3 @@
-a4cfea1797c928f2eff73573e559675d 
*tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
+79dcf3b32ed8e627a68bba19bf625ca5 
*tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
  tests/data/images/gbrp12le.dpx/%02d.gbrp12le.dpx CRC=0x1c755633
-609920 tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
+610304 tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
diff --git a/tests/ref/lavf/rgb48le.dpx b/tests/ref/lavf/rgb48le.dpx
index 073153898a..33817d95a9 100644
--- a/tests/ref/lavf/rgb48le.dpx
+++ b/tests/ref/lavf/rgb48le.dpx
@@ -1,3 +1,3 @@
-075963c3c08978b6a20555ba09161434 *tests/data/images/rgb48le.dpx/02.rgb48le.dpx
+51c703863c9df1db5ef78a77dd5fbd0f *tests/data/images/rgb48le.dpx/02.rgb48le.dpx
  tests/data/images/rgb48le.dpx/%02d.rgb48le.dpx CRC=0xe5b9c023
-609920 tests/data/images/rgb48le.dpx/02.rgb48le.dpx
+610304 tests/data/images/rgb48le.dpx/02.rgb48le.dpx
diff --git a/tests/ref/lavf/rgb48le_10.dpx b/tests/ref/lavf/rgb48le_10.dpx
index ce36e5079f..3c934168b7 100644
--- a/tests/ref/lavf/rgb48le_10.dpx
+++ b/tests/ref/lavf/rgb48le_10.dpx
@@ -1,3 +1,3 @@
-b9f22728f8ff393bf30cf6cbd624fa95 
*tests/data/images/rgb48le_10.dpx/02.rgb48le_10.dpx
+a0ca7132c33a5c0eb25dd4a2b6117743 
*tests/data/images/rgb48le_10.dpx/02.rgb48le_10.dpx
  tests/data/images/rgb48le_10.dpx/%02d.rgb48le_10.dpx CRC=0xf38d5830
-407168 tests/data/images/rgb48le_10.dpx/02.rgb48le_10.dpx
+407552 tests/data/images/rgb48le_10.dpx/02.rgb48le_10.dpx
diff --git a/tests/ref/lavf/rgba64le.dpx b/tests/ref/lavf/rgba64le.dpx
index b4092c9fd8..85974bbc98 100644
--- a/tests/ref/lavf/rgba64le.dpx
+++ b/tests/ref/lavf/rgba64le.dpx
@@ -1,3 +1,3 @@
-545603630f30dec2768c8ae8d12eb8ea 
*tests/data/images/rgba64le.dpx/02.rgba64le.dpx
+cb5fe2ad9c1119a33916a838cb586c45 
*tests/data/images/rgba64le.dpx/02.rgba64le.dpx
  

[FFmpeg-devel] [PATCH 2/2] avcodec/dpx_parser: export framerate and timebase of frames

[Paul B Mahol]

Also always skip pixel bytes.

Signed-off-by: Paul B Mahol 
---
 libavcodec/dpx_parser.c | 68 ++---
 1 file changed, 44 insertions(+), 24 deletions(-)

diff --git a/libavcodec/dpx_parser.c b/libavcodec/dpx_parser.c
index 8e4a01e09d..ed7b085040 100644
--- a/libavcodec/dpx_parser.c
+++ b/libavcodec/dpx_parser.c
@@ -32,6 +32,7 @@
 typedef struct DPXParseContext {
 ParseContext pc;
 uint32_t index;
+uint32_t hsize;
 uint32_t fsize;
 uint32_t remaining_size;
 int is_be;
@@ -43,14 +44,18 @@ static int dpx_parse(AVCodecParserContext *s, 
AVCodecContext *avctx,
 {
 DPXParseContext *d = s->priv_data;
 uint32_t state = d->pc.state;
-int next = END_NOT_FOUND;
-int i = 0;
+int next = END_NOT_FOUND, i = 0;
 
 s->pict_type = AV_PICTURE_TYPE_I;
+s->key_frame = 1;
 
 *poutbuf_size = 0;
-if (buf_size == 0)
-next = 0;
+*poutbuf = NULL;
+
+if (s->flags & PARSER_FLAG_COMPLETE_FRAMES) {
+next = buf_size;
+goto flush;
+}
 
 if (!d->pc.frame_start_found) {
 for (; i < buf_size; i++) {
@@ -64,36 +69,53 @@ static int dpx_parse(AVCodecParserContext *s, 
AVCodecContext *avctx,
 }
 }
 d->pc.state = state;
-} else {
-if (d->remaining_size) {
-i = FFMIN(d->remaining_size, buf_size);
-d->remaining_size -= i;
-if (d->remaining_size)
-goto flush;
+} else if (d->remaining_size) {
+i = FFMIN(d->remaining_size, buf_size);
+d->remaining_size -= i;
+if (!d->remaining_size) {
+d->pc.frame_start_found = 0;
 }
+goto flush;
 }
 
-for (; d->pc.frame_start_found && i < buf_size; i++) {
+for (; i < buf_size; i++) {
 d->pc.state = (d->pc.state << 8) | buf[i];
 d->index++;
-if (d->index == 17) {
+if (d->index == 5) {
+d->hsize = d->is_be ? d->pc.state : av_bswap32(d->pc.state);
+} else if (d->index == 17) {
 d->fsize = d->is_be ? d->pc.state : av_bswap32(d->pc.state);
-if (d->fsize <= 1664) {
+
+if (d->fsize <= 1664 || d->fsize <= d->hsize) {
 d->pc.frame_start_found = 0;
 goto flush;
 }
-if (d->fsize > buf_size - i + 19)
-d->remaining_size = d->fsize - buf_size + i - 19;
-else
-i += d->fsize - 19;
 
-break;
-} else if (d->index > 17) {
-if (d->pc.state == MKBETAG('S','D','P','X') ||
-d->pc.state == MKTAG('S','D','P','X')) {
-next = i - 3;
+if (d->hsize < 1728) {
+if (d->fsize > buf_size - i + 19) {
+d->remaining_size = d->fsize - buf_size + i - 19;
+} else {
+i += d->fsize - 19;
+}
 break;
 }
+} else if (d->index == 1725) {
+uint32_t fps = d->is_be ? d->pc.state : av_bswap32(d->pc.state);
+
+avctx->framerate = av_d2q(av_int2float(fps), 4096);
+avctx->time_base = av_inv_q(avctx->framerate);
+
+d->fsize -= d->index + 2;
+if (d->fsize > buf_size - i) {
+d->remaining_size = d->fsize - buf_size + i;
+} else {
+i += d->fsize;
+}
+break;
+} else if (state == MKBETAG('S','D','P','X') ||
+   state == MKTAG('S','D','P','X')) {
+next = i - 3;
+break;
 }
 }
 
@@ -101,8 +123,6 @@ flush:
 if (ff_combine_frame(>pc, next, , _size) < 0)
 return buf_size;
 
-d->pc.frame_start_found = 0;
-
 *poutbuf  = buf;
 *poutbuf_size = buf_size;
 return next;
-- 
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 1/2] avcodec/dpxenc: write framerate to header

[Paul B Mahol]

Signed-off-by: Paul B Mahol 
---
 libavcodec/dpxenc.c   | 21 ++---
 tests/ref/lavf/gbrp10le.dpx   |  4 ++--
 tests/ref/lavf/gbrp12le.dpx   |  4 ++--
 tests/ref/lavf/rgb48le.dpx|  4 ++--
 tests/ref/lavf/rgb48le_10.dpx |  4 ++--
 tests/ref/lavf/rgba64le.dpx   |  4 ++--
 6 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/libavcodec/dpxenc.c b/libavcodec/dpxenc.c
index a5960334d5..b296f9f22e 100644
--- a/libavcodec/dpxenc.c
+++ b/libavcodec/dpxenc.c
@@ -173,14 +173,25 @@ static void encode_gbrp12(AVCodecContext *avctx, const 
AVFrame *pic, uint16_t *d
 }
 }
 
+#define FILE_HEADER_SIZE 768
+#define IMAGE_HEADER_SIZE 640
+#define ORIENTATION_HEADER_SIZE 256
+#define FILM_INFO_HEADER_SIZE 256
+#define TV_INFO_HEADER_SIZE 128
+#define HEADER_SIZE (FILE_HEADER_SIZE + \
+ IMAGE_HEADER_SIZE + \
+ ORIENTATION_HEADER_SIZE + \
+ FILM_INFO_HEADER_SIZE + \
+ TV_INFO_HEADER_SIZE)
+
 static int encode_frame(AVCodecContext *avctx, AVPacket *pkt,
 const AVFrame *frame, int *got_packet)
 {
 DPXContext *s = avctx->priv_data;
-int size, ret, need_align, len;
+int ret, need_align, len;
+int64_t size;
 uint8_t *buf;
 
-#define HEADER_SIZE 1664  /* DPX Generic header */
 if (s->bits_per_component == 10)
 size = avctx->height * avctx->width * 4;
 else if (s->bits_per_component == 12) {
@@ -196,7 +207,7 @@ static int encode_frame(AVCodecContext *avctx, AVPacket 
*pkt,
 need_align = size - len;
 size *= avctx->height;
 }
-if ((ret = ff_alloc_packet2(avctx, pkt, size + HEADER_SIZE, 0)) < 0)
+if ((ret = ff_alloc_packet2(avctx, pkt, HEADER_SIZE + size, 0)) < 0)
 return ret;
 buf = pkt->data;
 
@@ -229,6 +240,10 @@ static int encode_frame(AVCodecContext *avctx, AVPacket 
*pkt,
 write32(buf + 1628, avctx->sample_aspect_ratio.num);
 write32(buf + 1632, avctx->sample_aspect_ratio.den);
 
+/* Film information header */
+if (avctx->framerate.num && avctx->framerate.den)
+write32(buf + 1724, av_float2int(av_q2d(avctx->framerate)));
+
 switch(s->bits_per_component) {
 case 8:
 case 16:
diff --git a/tests/ref/lavf/gbrp10le.dpx b/tests/ref/lavf/gbrp10le.dpx
index b33da34e20..7c03dc0779 100644
--- a/tests/ref/lavf/gbrp10le.dpx
+++ b/tests/ref/lavf/gbrp10le.dpx
@@ -1,3 +1,3 @@
-7ca935d5d5e00c54acbc85565d3039b6 
*tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
+c7c8ecd9d8c8a2c4dce6d92bfb9877d7 
*tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
 tests/data/images/gbrp10le.dpx/%02d.gbrp10le.dpx CRC=0xe6663fba
-407168 tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
+407552 tests/data/images/gbrp10le.dpx/02.gbrp10le.dpx
diff --git a/tests/ref/lavf/gbrp12le.dpx b/tests/ref/lavf/gbrp12le.dpx
index e2e794ecc6..5ca855b004 100644
--- a/tests/ref/lavf/gbrp12le.dpx
+++ b/tests/ref/lavf/gbrp12le.dpx
@@ -1,3 +1,3 @@
-a4cfea1797c928f2eff73573e559675d 
*tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
+79dcf3b32ed8e627a68bba19bf625ca5 
*tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
 tests/data/images/gbrp12le.dpx/%02d.gbrp12le.dpx CRC=0x1c755633
-609920 tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
+610304 tests/data/images/gbrp12le.dpx/02.gbrp12le.dpx
diff --git a/tests/ref/lavf/rgb48le.dpx b/tests/ref/lavf/rgb48le.dpx
index 073153898a..33817d95a9 100644
--- a/tests/ref/lavf/rgb48le.dpx
+++ b/tests/ref/lavf/rgb48le.dpx
@@ -1,3 +1,3 @@
-075963c3c08978b6a20555ba09161434 *tests/data/images/rgb48le.dpx/02.rgb48le.dpx
+51c703863c9df1db5ef78a77dd5fbd0f *tests/data/images/rgb48le.dpx/02.rgb48le.dpx
 tests/data/images/rgb48le.dpx/%02d.rgb48le.dpx CRC=0xe5b9c023
-609920 tests/data/images/rgb48le.dpx/02.rgb48le.dpx
+610304 tests/data/images/rgb48le.dpx/02.rgb48le.dpx
diff --git a/tests/ref/lavf/rgb48le_10.dpx b/tests/ref/lavf/rgb48le_10.dpx
index ce36e5079f..3c934168b7 100644
--- a/tests/ref/lavf/rgb48le_10.dpx
+++ b/tests/ref/lavf/rgb48le_10.dpx
@@ -1,3 +1,3 @@
-b9f22728f8ff393bf30cf6cbd624fa95 
*tests/data/images/rgb48le_10.dpx/02.rgb48le_10.dpx
+a0ca7132c33a5c0eb25dd4a2b6117743 
*tests/data/images/rgb48le_10.dpx/02.rgb48le_10.dpx
 tests/data/images/rgb48le_10.dpx/%02d.rgb48le_10.dpx CRC=0xf38d5830
-407168 tests/data/images/rgb48le_10.dpx/02.rgb48le_10.dpx
+407552 tests/data/images/rgb48le_10.dpx/02.rgb48le_10.dpx
diff --git a/tests/ref/lavf/rgba64le.dpx b/tests/ref/lavf/rgba64le.dpx
index b4092c9fd8..85974bbc98 100644
--- a/tests/ref/lavf/rgba64le.dpx
+++ b/tests/ref/lavf/rgba64le.dpx
@@ -1,3 +1,3 @@
-545603630f30dec2768c8ae8d12eb8ea 
*tests/data/images/rgba64le.dpx/02.rgba64le.dpx
+cb5fe2ad9c1119a33916a838cb586c45 
*tests/data/images/rgba64le.dpx/02.rgba64le.dpx
 tests/data/images/rgba64le.dpx/%02d.rgba64le.dpx CRC=0xe72ce131
-812672 tests/data/images/rgba64le.dpx/02.rgba64le.dpx
+813056 tests/data/images/rgba64le.dpx/02.rgba64le.dpx
-- 
2.17.1

___
ffmpeg-devel mailing 

Re: [FFmpeg-devel] [PATCH] Ticket #8750 Add inline function for the vec_xl intrinsic in non-VSX environments

[Andriy Gelman]

Hi Reimar,

On Sat, 20. Feb 12:34, Reimar Döffinger wrote:
> Hi!
> 
> > On 24 Jan 2021, at 17:35, Andriy Gelman  wrote:
> > 
> > On Sat, 07. Nov 16:31, Andriy Gelman wrote:
> >> On Sat, 31. Oct 10:17, Andriy Gelman wrote:
> >>> On Fri, 16. Oct 00:02, Andriy Gelman wrote:
>  On Fri, 09. Oct 20:17, Andriy Gelman wrote:
> > From: Chip Kerchner 
> > 
> > ---
> > libswscale/ppc/yuv2rgb_altivec.c | 10 ++
> > 1 file changed, 10 insertions(+)
> > 
> > diff --git a/libswscale/ppc/yuv2rgb_altivec.c 
> > b/libswscale/ppc/yuv2rgb_altivec.c
> > index 536545293d..930ef6b98f 100644
> > --- a/libswscale/ppc/yuv2rgb_altivec.c
> > +++ b/libswscale/ppc/yuv2rgb_altivec.c
> > @@ -283,6 +283,16 @@ static inline void cvtyuvtoRGB(SwsContext *c, 
> > vector signed short Y,
> >  * 
> > --
> >  */
> > 
> > +#if !HAVE_VSX
> > +static inline vector unsigned char vec_xl(signed long long offset, 
> > const ubyte *addr)
> > +{
> > +const vector unsigned char *v_addr = (const vector unsigned char 
> > *) (addr + offset);
> > +vector unsigned char align_perm = vec_lvsl(offset, addr);
> > +
> > +return (vector unsigned char) vec_perm(v_addr[0], v_addr[1], 
> > align_perm);
> > +}
> > +#endif /* !HAVE_VSX */
> > +
> > #define DEFCSP420_CVT(name, out_pixels) 
> >   \
> > static int altivec_ ## name(SwsContext *c, const unsigned char **in,
> >   \
> > int *instrides, int srcSliceY, int 
> > srcSliceH, \
>  
>  Ping.
>  This patch fixes PPC64 build on patchwork.
>  
> >>> 
> >>> ping
> >>> 
> >> 
> >> ping 
> >> 
> > 
> > I asked Reimar to have a look at the patch. He didn't have a link to 
> > original
> > post, so I'm forwarding his feedback: 
> > 
> > - a few comments sure would be good
> > - the function likely should be always_inline
> > - the offset is always 0 in the code, so that could be optimized
> > - I am not sure if the addresses even can be unaligned (the whole magic 
> > code is about fixing up unaligned loads since altivec simply ignores the 
> > low bits of the address, but it looks like the x86 asm also assumes 
> > unaligned)
> > - the extra load needed to fix the alignment can read outside the bounds of 
> > the buffer I think? Especially for chroma and if the load is aligned. 
> > Though chroma seems to have an issue already today...
> > 

> 
> I had a look at the code before the vec_xl was introduced, and besides the 
> unnecessary extra offset it seems it was pretty much like this.
> So worst case this patch would restore the behaviour to before the vec_xl 
> patch, which I
> guess is a reasonable argument to merge it whether it’s perfect or not.
> Personally I would have added a vecload (dropping the offset argument) or 
> similar function
> that either does vec_xl or this, instead of introducing a “fake” vec_xl 
> function,
> but that is just nitpicking I guess…
> 

Thanks for looking into this. 
Then I'll apply the patch in a few days if no one objects. 

I'll slightly reword the title/commit message:

lsws/ppc/yuv2rgb_altivec: Fix build in non-VSX environments

Fixes #8750
vec_xl intrinsic is only available on POWER 7 or higher. Add inline function to
fix builds if VSX is not supported.

-- 
Andriy
___
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 18/19] fate/matroska: Test remuxing tracks for hearing/visually impaired

[Andreas Rheinhardt]

The tests also test the other dispositions: commentary, descriptions
as well as dub and original language.

Signed-off-by: Andreas Rheinhardt 
---
The first version of the matroska-wtv-remux test used the fixed-point
mp2 decoder; given that no floats are involved I presumed that this
would give reproducible results among different arches. I was wrong [1].
Therefore this version uses only streamcopy.

I intend to apply this patchset tomorrow.

[1]: 
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20210217101356.1723370-18-andreas.rheinha...@gmail.com/

 tests/fate/matroska.mak  |  21 +
 tests/ref/fate/matroska-mpegts-remux |  52 +
 tests/ref/fate/matroska-wtv-remux| 111 +++
 3 files changed, 184 insertions(+)
 create mode 100644 tests/ref/fate/matroska-mpegts-remux
 create mode 100644 tests/ref/fate/matroska-wtv-remux

diff --git a/tests/fate/matroska.mak b/tests/fate/matroska.mak
index 4c10fe663f..74461107c9 100644
--- a/tests/fate/matroska.mak
+++ b/tests/fate/matroska.mak
@@ -100,6 +100,27 @@ FATE_MATROSKA_FFMPEG_FFPROBE-$(call ALLYES, FILE_PROTOCOL 
MATROSKA_DEMUXER \
+= fate-matroska-vp8-alpha-remux
 fate-matroska-vp8-alpha-remux: CMD = transcode matroska 
$(TARGET_SAMPLES)/vp8_alpha/vp8_video_with_alpha.webm matroska "-c copy 
-disposition +hearing_impaired -cluster_size_limit 10" "-c copy -t 0.2" "" 
"-show_entries stream_disposition:stream_side_data_list"
 
+# The sample has two audio tracks which are both streamcopied twice with
+# different dispositions. The first input track is muxed once marked as
+# original and once marked as dub; the second input audio track is already
+# marked as being for the hearing impaired. It is muxed once as-is and once
+# additionally marked as comment.
+FATE_MATROSKA_FFMPEG_FFPROBE-$(call ALLYES, FILE_PROTOCOL WTV_DEMUXER   \
+MPEGAUDIO_PARSER DVBSUB_PARSER  \
+MP2_DECODER MATROSKA_MUXER  \
+MATROSKA_DEMUXER FRAMECRC_MUXER \
+PIPE_PROTOCOL)  \
+   += fate-matroska-wtv-remux
+fate-matroska-wtv-remux: CMD = transcode wtv 
$(TARGET_SAMPLES)/wtv/law-and-order-partial.wtv matroska "-map 0 -vn -map 0:a:1 
-c copy -disposition:s +descriptions -disposition:a:0 +original 
-disposition:a:1 +comment -map 0:a:0 -disposition:a:3 +dub" "-map 0:0 -map 0:1 
-map 0:2 -c copy -t 0.2" "" "-show_entries 
stream_disposition:stream=index,codec_name"
+
+# The audio stream to be remuxed here has AV_DISPOSITION_VISUAL_IMPAIRED.
+FATE_MATROSKA_FFMPEG_FFPROBE-$(call ALLYES, FILE_PROTOCOL MPEGTS_DEMUXER\
+AC3_DECODER MATROSKA_MUXER  \
+MATROSKA_DEMUXER FRAMECRC_MUXER \
+PIPE_PROTOCOL)  \
+   += fate-matroska-mpegts-remux
+fate-matroska-mpegts-remux: CMD = transcode mpegts 
$(TARGET_SAMPLES)/mpegts/pmtchange.ts matroska "-map 0:2 -map 0:2 -c copy 
-disposition:a:1 -visual_impaired+hearing_impaired" "-map 0 -c copy" "" 
"-show_entries stream_disposition:stream=index"
+
 FATE_MATROSKA_FFPROBE-$(call ALLYES, MATROSKA_DEMUXER) += 
fate-matroska-spherical-mono
 fate-matroska-spherical-mono: CMD = run ffprobe$(PROGSSUF)$(EXESUF) 
-show_entries stream_side_data_list -select_streams v -v 0 
$(TARGET_SAMPLES)/mkv/spherical.mkv
 
diff --git a/tests/ref/fate/matroska-mpegts-remux 
b/tests/ref/fate/matroska-mpegts-remux
new file mode 100644
index 00..3c0b41cca9
--- /dev/null
+++ b/tests/ref/fate/matroska-mpegts-remux
@@ -0,0 +1,52 @@
+4e6253c1f5f96ff64ae855dea426547d 
*tests/data/fate/matroska-mpegts-remux.matroska
+6509 tests/data/fate/matroska-mpegts-remux.matroska
+#tb 0: 1/1000
+#media_type 0: audio
+#codec_id 0: ac3
+#sample_rate 0: 48000
+#channel_layout 0: 3
+#channel_layout_name 0: stereo
+#tb 1: 1/1000
+#media_type 1: audio
+#codec_id 1: ac3
+#sample_rate 1: 48000
+#channel_layout 1: 3
+#channel_layout_name 1: stereo
+0,  0,  0,   32,  768, 0xa63778d4
+1,  0,  0,   32,  768, 0xa63778d4
+0, 32, 32,   32,  768, 0x7d577f3f
+1, 32, 32,   32,  768, 0x7d577f3f
+0, 64, 64,   32,  768, 0xd86b7c8f
+1, 64, 64,   32,  768, 0xd86b7c8f
+0, 96, 96,   32,  626, 0x09f4382f
+1, 96, 96,   32,  626, 0x09f4382f
+[STREAM]
+index=0
+DISPOSITION:default=1
+DISPOSITION:dub=0
+DISPOSITION:original=0
+DISPOSITION:comment=0
+DISPOSITION:lyrics=0
+DISPOSITION:karaoke=0
+DISPOSITION:forced=0
+DISPOSITION:hearing_impaired=0
+DISPOSITION:visual_impaired=1
+DISPOSITION:clean_effects=0
+DISPOSITION:attached_pic=0

Re: [FFmpeg-devel] [PATCH 2/4] libavutil: add side data AV_FRAME_DATA_BOUNDING_BOXES

[Lynne]

Feb 20, 2021, 15:13 by yejun@intel.com:

>
>
>> -Original Message-
>> From: ffmpeg-devel  On Behalf Of Lynne
>> Sent: 2021年2月20日 6:54
>> To: FFmpeg development discussions and patches 
>> Subject: Re: [FFmpeg-devel] [PATCH 2/4] libavutil: add side data
>> AV_FRAME_DATA_BOUNDING_BOXES
>>
>> Feb 19, 2021, 07:59 by yejun@intel.com:
>>
>> > Signed-off-by: Guo, Yejun 
>> > ---
>> >  doc/APIchanges  | 2 ++
>> >  libavutil/frame.c   | 1 +
>> >  libavutil/frame.h   | 7 +++
>> >  libavutil/version.h | 2 +-
>> >  4 files changed, 11 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/doc/APIchanges b/doc/APIchanges
>> > index c353d2d281..3c6e9e351e 100644
>> > --- a/doc/APIchanges
>> > +++ b/doc/APIchanges
>> > @@ -14,6 +14,8 @@ libavutil: 2017-10-21
>> >
>>
>> This won't really work.
>> Side data must be public but the header it's specified in is neither 
>> installed,
>> neither in lavu. There's no such thing as ffmpeg-only private data.
>>
>> If we do plan to make this side data public, it'll require a lot more
>> considerations,
>> and versioning like with the film grain side data, since as you pointed out 
>> it
>> may
>> be used by codecs.
>>
>
> Thanks for your feedback.
>
> Could you share a bit more about the 'versioning' of film grain? thanks.
>
> I checked file film_grain_params.h/c and do not find it is version relative.
>
> As for the bounding box, the BoundingBoxHeader.bbox_size acts like
> a version checker. (different bbox_size if the struct is changed later)
>

That's not really a good solution. I meant having a dedicated and documented
version field which switches structs.
Changing the struct size means we can only add fields, so if we need to alter
the behavior of previous fields we'll end up with a data format more convoluted
than mpeg audio codecs.


>> For now, although it's somewhat less appropriate, you can replace
>> the side data with frame->private_ref. It's an AVBufferRef that's for
>> use by a single internal library, and we don't use it anywhere except
>> in lavc for now.
>>
>
> Thanks for pointing this possible way, I'll investigate it.
> I would prefer side data which is possible the final solution.
>

I'd rather not commit us to a side data format that may change
in the future and has no lavfi-external use. Could you implement it as a
private_ref, at least for the time being? It's lavfi-only, so it makes sense
for now, plus we can keep changing it, and once an external user
appears we can make it proper public side data?
___
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 137/217] avcodec/cpia: Mark decoder as init-threadsafe

[Stephan Hilb]

> libavcodec/cpia.c | 1 +
> 1 file changed, 1 insertion(+)

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 2/6] lavc: split LSCR decoder out of PNG decoder

[James Almer]

On 2/16/2021 5:24 PM, Anton Khirnov wrote:

It shares very little code with pngdec, so keeping them together only
makes the code harder to read.
---
  libavcodec/Makefile  |   2 +-
  libavcodec/lscrdec.c | 279 +++
  libavcodec/png.h |   5 +
  libavcodec/pngdec.c  | 174 +--
  4 files changed, 291 insertions(+), 169 deletions(-)
  create mode 100644 libavcodec/lscrdec.c

diff --git a/libavcodec/Makefile b/libavcodec/Makefile
index 3341801b97..b0c6d675d0 100644
--- a/libavcodec/Makefile
+++ b/libavcodec/Makefile
@@ -439,7 +439,7 @@ OBJS-$(CONFIG_KMVC_DECODER)+= kmvc.o
  OBJS-$(CONFIG_LAGARITH_DECODER)+= lagarith.o lagarithrac.o
  OBJS-$(CONFIG_LJPEG_ENCODER)   += ljpegenc.o mjpegenc_common.o
  OBJS-$(CONFIG_LOCO_DECODER)+= loco.o
-OBJS-$(CONFIG_LSCR_DECODER)+= png.o pngdec.o pngdsp.o
+OBJS-$(CONFIG_LSCR_DECODER)+= lscrdec.o png.o pngdec.o pngdsp.o
  OBJS-$(CONFIG_M101_DECODER)+= m101.o
  OBJS-$(CONFIG_MACE3_DECODER)   += mace.o
  OBJS-$(CONFIG_MACE6_DECODER)   += mace.o
diff --git a/libavcodec/lscrdec.c b/libavcodec/lscrdec.c
new file mode 100644
index 00..242ae8fcb2
--- /dev/null
+++ b/libavcodec/lscrdec.c
@@ -0,0 +1,279 @@
+/*
+ * 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 
+#include 
+
+#include "libavutil/frame.h"
+#include "libavutil/error.h"
+#include "libavutil/log.h"
+
+#include "avcodec.h"
+#include "bytestream.h"
+#include "codec.h"
+#include "internal.h"
+#include "packet.h"
+#include "png.h"
+#include "pngdsp.h"
+
+typedef struct LSCRContext {
+PNGDSPContext   dsp;
+AVCodecContext *avctx;
+
+AVFrame*last_picture;
+uint8_t*buffer;
+int buffer_size;
+uint8_t*crow_buf;
+int crow_size;
+uint8_t*last_row;
+unsigned intlast_row_size;
+
+GetByteContext  gb;
+uint8_t*image_buf;
+int image_linesize;
+int row_size;
+int cur_h;
+int y;
+
+z_streamzstream;
+} LSCRContext;
+
+static void handle_row(LSCRContext *s, AVFrame *frame)


Unused frame argument.


+{
+uint8_t *ptr, *last_row;
+
+ptr = s->image_buf + s->image_linesize * s->y;
+if (s->y == 0)
+last_row = s->last_row;
+else
+last_row = ptr - s->image_linesize;
+
+ff_png_filter_row(>dsp, ptr, s->crow_buf[0], s->crow_buf + 1,
+  last_row, s->row_size, 3);
+
+s->y++;
+}
+
+static int decode_idat(LSCRContext *s, AVFrame *frame, int length)
+{
+int ret;
+s->zstream.avail_in = FFMIN(length, bytestream2_get_bytes_left(>gb));
+s->zstream.next_in  = s->gb.buffer;
+bytestream2_skip(>gb, length);
+
+/* decode one line if possible */
+while (s->zstream.avail_in > 0) {
+ret = inflate(>zstream, Z_PARTIAL_FLUSH);
+if (ret != Z_OK && ret != Z_STREAM_END) {
+av_log(s->avctx, AV_LOG_ERROR, "inflate returned error %d\n", ret);
+return AVERROR_EXTERNAL;
+}
+if (s->zstream.avail_out == 0) {
+if (s->y < s->cur_h) {
+handle_row(s, frame);
+}
+s->zstream.avail_out = s->crow_size;
+s->zstream.next_out  = s->crow_buf;
+}
+if (ret == Z_STREAM_END && s->zstream.avail_in > 0) {
+av_log(s->avctx, AV_LOG_WARNING,
+   "%d undecompressed bytes left in buffer\n", 
s->zstream.avail_in);
+return 0;
+}
+}
+return 0;
+}
+
+static int decode_frame_lscr(AVCodecContext *avctx,
+ void *data, int *got_frame,
+ AVPacket *avpkt)
+{
+LSCRContext *const s = avctx->priv_data;
+GetByteContext *gb = >gb;
+AVFrame *frame = data;
+int ret, nb_blocks, offset = 0;
+
+if (avpkt->size < 2)
+return AVERROR_INVALIDDATA;
+if (avpkt->size == 2)
+return 0;
+
+bytestream2_init(gb, avpkt->data, avpkt->size);
+
+if ((ret = ff_get_buffer(avctx, frame, AV_GET_BUFFER_FLAG_REF)) < 0)
+return ret;
+
+nb_blocks = bytestream2_get_le16(gb);
+if 

Re: [FFmpeg-devel] [PATCH] avcodec: remove pointless lowres deprecation wrappers

[James Almer]

On 2/13/2021 1:32 PM, James Almer wrote:

Neither the feature, public fields, or AVOptions were ever truly deprecated,
and will also not be removed if this FF_API_ define was left in place, so
remove it as it's misleading.

Signed-off-by: James Almer 
---
  libavcodec/avcodec.h | 13 -
  libavcodec/version.h |  3 ---
  libavformat/utils.c  |  2 --
  3 files changed, 18 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 7dbf083a24..5df6a8aedc 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -1741,14 +1741,12 @@ typedef struct AVCodecContext {
   */
  int bits_per_raw_sample;
  
-#if FF_API_LOWRES

  /**
   * low resolution decoding, 1-> 1/2 size, 2->1/4 size
   * - encoding: unused
   * - decoding: Set by user.
   */
   int lowres;
-#endif
  
  #if FF_API_CODED_FRAME

  /**
@@ -2084,15 +2082,6 @@ typedef struct AVCodecContext {
   */
  const AVCodecDescriptor *codec_descriptor;
  
-#if !FF_API_LOWRES

-/**
- * low resolution decoding, 1-> 1/2 size, 2->1/4 size
- * - encoding: unused
- * - decoding: Set by user.
- */
- int lowres;
-#endif
-
  /**
   * Current statistics for PTS correction.
   * - decoding: maintained and used by libavcodec, not intended to be used 
by user apps
@@ -2366,12 +2355,10 @@ void 
av_codec_set_codec_descriptor(AVCodecContext *avctx, co
  attribute_deprecated
  unsigned av_codec_get_codec_properties(const AVCodecContext *avctx);
  
-#if FF_API_LOWRES

  attribute_deprecated
  int  av_codec_get_lowres(const AVCodecContext *avctx);
  attribute_deprecated
  void av_codec_set_lowres(AVCodecContext *avctx, int val);
-#endif
  
  attribute_deprecated

  int  av_codec_get_seek_preroll(const AVCodecContext *avctx);
diff --git a/libavcodec/version.h b/libavcodec/version.h
index 83dbd1ad63..1eb140a6c9 100644
--- a/libavcodec/version.h
+++ b/libavcodec/version.h
@@ -51,9 +51,6 @@
   * at once through the bump. This improves the git bisect-ability of the 
change.
   */
  
-#ifndef FF_API_LOWRES

-#define FF_API_LOWRES(LIBAVCODEC_VERSION_MAJOR < 59)
-#endif
  #ifndef FF_API_AVCTX_TIMEBASE
  #define FF_API_AVCTX_TIMEBASE(LIBAVCODEC_VERSION_MAJOR < 59)
  #endif
diff --git a/libavformat/utils.c b/libavformat/utils.c
index 3e955b85bc..5d40678325 100644
--- a/libavformat/utils.c
+++ b/libavformat/utils.c
@@ -4130,7 +4130,6 @@ FF_DISABLE_DEPRECATION_WARNINGS
  if (ret < 0)
  goto find_stream_info_err;
  
-#if FF_API_LOWRES

  // The old API (AVStream.codec) "requires" the resolution to be 
adjusted
  // by the lowres factor.
  if (st->internal->avctx->lowres && st->internal->avctx->width) {
@@ -4138,7 +4137,6 @@ FF_DISABLE_DEPRECATION_WARNINGS
  st->codec->width = st->internal->avctx->width;
  st->codec->height = st->internal->avctx->height;
  }
-#endif
  
  if (st->codec->codec_tag != MKTAG('t','m','c','d')) {

  st->codec->time_base = st->internal->avctx->time_base;


Will apply soon.
___
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 16/16] avcodec/rv34data: Remove rv34_dquant_tab

[Andreas Rheinhardt]

It is unused and coincides with ff_modified_quant_tab.

Signed-off-by: Andreas Rheinhardt 
---
 libavcodec/rv34data.h | 13 -
 1 file changed, 13 deletions(-)

diff --git a/libavcodec/rv34data.h b/libavcodec/rv34data.h
index 32ecc395a8..4509e1feed 100644
--- a/libavcodec/rv34data.h
+++ b/libavcodec/rv34data.h
@@ -99,19 +99,6 @@ static const uint8_t rv34_quant_to_vlc_set[2][32] = {
3, 3, 3, 4, 4, 4, 4, 5, 5, 5, 5, 6, 6, 6, 6, 6 },
 };
 
-/**
- * table for obtaining the quantizer difference
- * @todo Use with ff_modified_quant_tab from h263data.h.
- */
-static const uint8_t rv34_dquant_tab[2][32]={
-//  0  1  2  3  4  5  6  7  8  9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 
25 26 27 28 29 30 31
-{
-0, 3, 1, 2, 3, 4, 5, 6, 7, 8, 9, 
9,10,11,12,13,14,15,16,17,18,18,19,20,21,22,23,24,25,26,27,28
-},{
-0, 2, 3, 4, 5, 6, 7, 8, 
9,10,11,13,14,15,16,17,18,19,20,21,22,24,25,26,27,28,29,30,31,31,31,26
-}
-};
-
 /**
  * maximum number of macroblocks for each of the possible slice offset sizes
  * @todo This is the same as ff_mba_max, maybe use it instead.
-- 
2.27.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".

Re: [FFmpeg-devel] [PATCH 2/4] libavutil: add side data AV_FRAME_DATA_BOUNDING_BOXES

[Guo, Yejun]

> -Original Message-
> From: ffmpeg-devel  On Behalf Of Lynne
> Sent: 2021年2月20日 6:54
> To: FFmpeg development discussions and patches 
> Subject: Re: [FFmpeg-devel] [PATCH 2/4] libavutil: add side data
> AV_FRAME_DATA_BOUNDING_BOXES
> 
> Feb 19, 2021, 07:59 by yejun@intel.com:
> 
> > Signed-off-by: Guo, Yejun 
> > ---
> >  doc/APIchanges  | 2 ++
> >  libavutil/frame.c   | 1 +
> >  libavutil/frame.h   | 7 +++
> >  libavutil/version.h | 2 +-
> >  4 files changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/doc/APIchanges b/doc/APIchanges
> > index c353d2d281..3c6e9e351e 100644
> > --- a/doc/APIchanges
> > +++ b/doc/APIchanges
> > @@ -14,6 +14,8 @@ libavutil: 2017-10-21
> >
> 
> This won't really work.
> Side data must be public but the header it's specified in is neither 
> installed,
> neither in lavu. There's no such thing as ffmpeg-only private data.
> 
> If we do plan to make this side data public, it'll require a lot more
> considerations,
> and versioning like with the film grain side data, since as you pointed out it
> may
> be used by codecs.

Thanks for your feedback.

Could you share a bit more about the 'versioning' of film grain? thanks.

I checked file film_grain_params.h/c and do not find it is version relative.

As for the bounding box, the BoundingBoxHeader.bbox_size acts like
a version checker. (different bbox_size if the struct is changed later)

> 
> For now, although it's somewhat less appropriate, you can replace
> the side data with frame->private_ref. It's an AVBufferRef that's for
> use by a single internal library, and we don't use it anywhere except
> in lavc for now.

Thanks for pointing this possible way, I'll investigate it.
I would prefer side data which is possible the final solution.

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

[FFmpeg-devel] [PATCH] avutil/buffer: free all pooled buffers immediately after uninitializing the pool

[James Almer]

No buffer will be fetched from the pool after it's uninitialized, so there's
no benefit from waiting until every single buffer has been returned to it
before freeing them all.
This should free some memory in certain scenarios, which can be beneficial in
low memory systems.

Based on a patch by Jonas Karlman.

Signed-off-by: James Almer 
---
 libavutil/buffer.c | 19 ++-
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/libavutil/buffer.c b/libavutil/buffer.c
index d67b4bbdaf..3204f11b68 100644
--- a/libavutil/buffer.c
+++ b/libavutil/buffer.c
@@ -279,11 +279,7 @@ AVBufferPool *av_buffer_pool_init(int size, AVBufferRef* 
(*alloc)(int size))
 return pool;
 }
 
-/*
- * This function gets called when the pool has been uninited and
- * all the buffers returned to it.
- */
-static void buffer_pool_free(AVBufferPool *pool)
+static void buffer_pool_flush(AVBufferPool *pool)
 {
 while (pool->pool) {
 BufferPoolEntry *buf = pool->pool;
@@ -292,6 +288,15 @@ static void buffer_pool_free(AVBufferPool *pool)
 buf->free(buf->opaque, buf->data);
 av_freep();
 }
+}
+
+/*
+ * This function gets called when the pool has been uninited and
+ * all the buffers returned to it.
+ */
+static void buffer_pool_free(AVBufferPool *pool)
+{
+buffer_pool_flush(pool);
 ff_mutex_destroy(>mutex);
 
 if (pool->pool_free)
@@ -309,6 +314,10 @@ void av_buffer_pool_uninit(AVBufferPool **ppool)
 pool   = *ppool;
 *ppool = NULL;
 
+ff_mutex_lock(>mutex);
+buffer_pool_flush(pool);
+ff_mutex_unlock(>mutex);
+
 if (atomic_fetch_sub_explicit(>refcount, 1, memory_order_acq_rel) == 
1)
 buffer_pool_free(pool);
 }
-- 
2.30.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".

Re: [FFmpeg-devel] [PATCH 31/32] avcodec/ac3tab: Move ff_ac3_enc_channel_map to its only user

[Lynne]

Feb 20, 2021, 06:01 by andreas.rheinha...@gmail.com:

> and make it static.
>
> Signed-off-by: Andreas Rheinhardt 
> ---
>  libavcodec/ac3enc.c | 11 ++-
>  libavcodec/ac3tab.c | 18 --
>  libavcodec/ac3tab.h |  9 -
>  3 files changed, 18 insertions(+), 20 deletions(-)
>
> diff --git a/libavcodec/ac3enc.c b/libavcodec/ac3enc.c
> index bae7405fff..65602d2021 100644
> --- a/libavcodec/ac3enc.c
> +++ b/libavcodec/ac3enc.c
> @@ -161,6 +161,15 @@ const uint64_t ff_ac3_channel_layouts[19] = {
>  0
>  };
>  
> +/**
> + * Table to remap channels from SMPTE order to AC-3 order.
> + * [channel_mode][lfe][ch]
> + */
> +static const uint8_t ac3_enc_channel_map[8][2][6] = {
> +COMMON_CHANNEL_MAP
> +{ { 0, 1, 2, 3,}, { 0, 1, 3, 4, 2,   } },
> +{ { 0, 2, 1, 3, 4, }, { 0, 2, 1, 4, 5, 3 } },
> +};
>  
>  /**
>  * LUT to select the bandwidth code based on the bit rate, sample rate, and
> @@ -2184,7 +2193,7 @@ static av_cold int set_channel_info(AC3EncodeContext 
> *s, int channels,
>  s->has_center   = (s->channel_mode & 0x01) && s->channel_mode != 
> AC3_CHMODE_MONO;
>  s->has_surround =  s->channel_mode & 0x04;
>  
> -s->channel_map  = ff_ac3_enc_channel_map[s->channel_mode][s->lfe_on];
> +s->channel_map  = ac3_enc_channel_map[s->channel_mode][s->lfe_on];
>  *channel_layout = ch_layout;
>  if (s->lfe_on)
>  *channel_layout |= AV_CH_LOW_FREQUENCY;
> diff --git a/libavcodec/ac3tab.c b/libavcodec/ac3tab.c
> index 99307218cc..5a352340e7 100644
> --- a/libavcodec/ac3tab.c
> +++ b/libavcodec/ac3tab.c
> @@ -97,24 +97,6 @@ const uint16_t avpriv_ac3_channel_layout_tab[8] = {
>  AV_CH_LAYOUT_5POINT0
>  };
>  
> -#define COMMON_CHANNEL_MAP \
> -{ { 0, 1,  }, { 0, 1, 2, } },\
> -{ { 0, }, { 0, 1,} },\
> -{ { 0, 1,  }, { 0, 1, 2, } },\
> -{ { 0, 2, 1,   }, { 0, 2, 1, 3,  } },\
> -{ { 0, 1, 2,   }, { 0, 1, 3, 2,  } },\
> -{ { 0, 2, 1, 3,}, { 0, 2, 1, 4, 3,   } },
> -
> -/**
> - * Table to remap channels from SMPTE order to AC-3 order.
> - * [channel_mode][lfe][ch]
> - */
> -const uint8_t ff_ac3_enc_channel_map[8][2][6] = {
> -COMMON_CHANNEL_MAP
> -{ { 0, 1, 2, 3,}, { 0, 1, 3, 4, 2,   } },
> -{ { 0, 2, 1, 3, 4, }, { 0, 2, 1, 4, 5, 3 } },
> -};
> -
>  /**
>  * Table to remap channels from AC-3 order to SMPTE order.
>  * [channel_mode][lfe][ch]
> diff --git a/libavcodec/ac3tab.h b/libavcodec/ac3tab.h
> index a0036a301b..f41f7b6da0 100644
> --- a/libavcodec/ac3tab.h
> +++ b/libavcodec/ac3tab.h
> @@ -31,7 +31,6 @@
>  extern const uint16_t ff_ac3_frame_size_tab[38][3];
>  extern const uint8_t  ff_ac3_channels_tab[8];
>  extern av_export_avcodec const uint16_t avpriv_ac3_channel_layout_tab[8];
> -extern const uint8_t  ff_ac3_enc_channel_map[8][2][6];
>  extern const uint8_t  ff_ac3_dec_channel_map[8][2][6];
>  extern const int  ff_ac3_sample_rate_tab[];
>  extern const uint16_t ff_ac3_bitrate_tab[19];
> @@ -67,4 +66,12 @@ enum CustomChannelMapLocation{
>  AC3_CHMAP_LFE = 1<<(15-15)
>  };
>  
> +#define COMMON_CHANNEL_MAP \
> +{ { 0, 1,  }, { 0, 1, 2, } },\
> +{ { 0, }, { 0, 1,} },\
> +{ { 0, 1,  }, { 0, 1, 2, } },\
> +{ { 0, 2, 1,   }, { 0, 2, 1, 3,  } },\
> +{ { 0, 1, 2,   }, { 0, 1, 3, 2,  } },\
> +{ { 0, 2, 1, 3,}, { 0, 2, 1, 4, 3,   } },
> +
>  #endif /* AVCODEC_AC3TAB_H */
> -- 
> 2.27.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".
>

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 32/32] avcodec/dirac_vlc: Make ff_dirac_golomb_lut static

[Lynne]

Feb 20, 2021, 06:01 by andreas.rheinha...@gmail.com:

> Only used here.
>
> Signed-off-by: Andreas Rheinhardt 
>

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] Ticket #8750 Add inline function for the vec_xl intrinsic in non-VSX environments

[Reimar Döffinger]

Hi!

> On 24 Jan 2021, at 17:35, Andriy Gelman  wrote:
> 
> On Sat, 07. Nov 16:31, Andriy Gelman wrote:
>> On Sat, 31. Oct 10:17, Andriy Gelman wrote:
>>> On Fri, 16. Oct 00:02, Andriy Gelman wrote:
 On Fri, 09. Oct 20:17, Andriy Gelman wrote:
> From: Chip Kerchner 
> 
> ---
> libswscale/ppc/yuv2rgb_altivec.c | 10 ++
> 1 file changed, 10 insertions(+)
> 
> diff --git a/libswscale/ppc/yuv2rgb_altivec.c 
> b/libswscale/ppc/yuv2rgb_altivec.c
> index 536545293d..930ef6b98f 100644
> --- a/libswscale/ppc/yuv2rgb_altivec.c
> +++ b/libswscale/ppc/yuv2rgb_altivec.c
> @@ -283,6 +283,16 @@ static inline void cvtyuvtoRGB(SwsContext *c, vector 
> signed short Y,
>  * 
> --
>  */
> 
> +#if !HAVE_VSX
> +static inline vector unsigned char vec_xl(signed long long offset, const 
> ubyte *addr)
> +{
> +const vector unsigned char *v_addr = (const vector unsigned char *) 
> (addr + offset);
> +vector unsigned char align_perm = vec_lvsl(offset, addr);
> +
> +return (vector unsigned char) vec_perm(v_addr[0], v_addr[1], 
> align_perm);
> +}
> +#endif /* !HAVE_VSX */
> +
> #define DEFCSP420_CVT(name, out_pixels)   
> \
> static int altivec_ ## name(SwsContext *c, const unsigned char **in,  
> \
> int *instrides, int srcSliceY, int srcSliceH, 
> \
 
 Ping.
 This patch fixes PPC64 build on patchwork.
 
>>> 
>>> ping
>>> 
>> 
>> ping 
>> 
> 
> I asked Reimar to have a look at the patch. He didn't have a link to original
> post, so I'm forwarding his feedback: 
> 
> - a few comments sure would be good
> - the function likely should be always_inline
> - the offset is always 0 in the code, so that could be optimized
> - I am not sure if the addresses even can be unaligned (the whole magic code 
> is about fixing up unaligned loads since altivec simply ignores the low bits 
> of the address, but it looks like the x86 asm also assumes unaligned)
> - the extra load needed to fix the alignment can read outside the bounds of 
> the buffer I think? Especially for chroma and if the load is aligned. Though 
> chroma seems to have an issue already today...
> 

I had a look at the code before the vec_xl was introduced, and besides the 
unnecessary extra offset it seems it was pretty much like this.
So worst case this patch would restore the behaviour to before the vec_xl 
patch, which I
guess is a reasonable argument to merge it whether it’s perfect or not.
Personally I would have added a vecload (dropping the offset argument) or 
similar function
that either does vec_xl or this, instead of introducing a “fake” vec_xl 
function,
but that is just nitpicking I guess…

Best regards,
Reimar

___
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: add Simbiosis IMX video decoder

[Paul B Mahol]

will apply soon this set
___
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".