Re: [FFmpeg-devel] [PATCH v5 01/10] channel_layout: add new channel positions supported by xHE-AAC

2024-05-31 Thread Jan Ekström
On Thu, May 30, 2024 at 5:39 AM Lynne via ffmpeg-devel
 wrote:
>
> apichanges will be updated upon merging, as well as a version bump.
> ---
>  libavutil/channel_layout.c | 4 
>  libavutil/channel_layout.h | 8 
>  2 files changed, 12 insertions(+)
>
> diff --git a/libavutil/channel_layout.c b/libavutil/channel_layout.c
> index 98839b7250..2d6963b6df 100644
> --- a/libavutil/channel_layout.c
> +++ b/libavutil/channel_layout.c
> @@ -75,6 +75,10 @@ static const struct channel_name channel_names[] = {
>  [AV_CHAN_BOTTOM_FRONT_CENTER  ] = { "BFC",   "bottom front center"   
> },
>  [AV_CHAN_BOTTOM_FRONT_LEFT] = { "BFL",   "bottom front left" 
> },
>  [AV_CHAN_BOTTOM_FRONT_RIGHT   ] = { "BFR",   "bottom front right"
> },
> +[AV_CHAN_SIDE_SURROUND_LEFT   ] = { "SSL",   "side surround left"
> },
> +[AV_CHAN_SIDE_SURROUND_RIGHT  ] = { "SSR",   "side surround right"   
> },
> +[AV_CHAN_TOP_SURROUND_LEFT] = { "TTL",   "top surround left" 
> },
> +[AV_CHAN_TOP_SURROUND_RIGHT   ] = { "TTR",   "top surround right"
> },

Just noticed for these two top ones, is there a connection being "TTL"
and "top surround left" that I somehow missed, or is this a typo of
"TSL"?

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


Re: [FFmpeg-devel] [PATCH v2 1/6] lavf/tls_mbedtls: handle more error codes for

2024-06-03 Thread Jan Ekström
On Wed, May 29, 2024 at 2:05 PM sfan5  wrote:
>

Did an initial tired look at the set, and in general it looks alright
and the wrapper still builds with Fedora's mbedtls 2.28.8.

(Of course then it fails to link due to unchecked usage of
`mbedtls_x509_crt_{init,free,parse_file}` in tls_mbedtls, as well as
`mbedtls_mpi_copy` in rtmpdh. But this breakage is unrelated to this
patch, as current master does exactly the same)

I'd just probably move the MBEDTLS_ERR_X509_CERT_VERIFY_FAILED logging
diff into the first commit that adds error codes (also probably
"messages" in the commit message there?), as adding that error's
logging really doesn't have anything to do with the verify=0 + TLS 1.3
workaround.

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


Re: [FFmpeg-devel] [PATCH 1/3] avutil/ambient_viewing_environment: set a sane default value for AVRational fields

2024-06-21 Thread Jan Ekström
On Thu, Jun 20, 2024 at 6:54 PM James Almer  wrote:
>
> On 6/18/2024 4:20 PM, James Almer wrote:
> > Prevent potential divisions by 0 when using them immediately after 
> > allocation.
> >
> > Signed-off-by: James Almer 
> > ---
> >   libavutil/ambient_viewing_environment.c | 10 ++
> >   1 file changed, 10 insertions(+)
> >
> > diff --git a/libavutil/ambient_viewing_environment.c 
> > b/libavutil/ambient_viewing_environment.c
> > index c47458cfa8..e359727776 100644
> > --- a/libavutil/ambient_viewing_environment.c
> > +++ b/libavutil/ambient_viewing_environment.c
> > @@ -21,6 +21,13 @@
> >   #include "ambient_viewing_environment.h"
> >   #include "mem.h"
> >
> > +static void get_defaults(AVAmbientViewingEnvironment *env)
> > +{
> > +env->ambient_illuminance =
> > +env->ambient_light_x =
> > +env->ambient_light_y = (AVRational) { 0, 1 };
> > +}
> > +
> >   AVAmbientViewingEnvironment *av_ambient_viewing_environment_alloc(size_t 
> > *size)
> >   {
> >   AVAmbientViewingEnvironment *env =
> > @@ -28,6 +35,8 @@ AVAmbientViewingEnvironment 
> > *av_ambient_viewing_environment_alloc(size_t *size)
> >   if (!env)
> >   return NULL;
> >
> > +get_defaults(env);
> > +
> >if (size)
> >   *size = sizeof(*env);
> >
> > @@ -44,6 +53,7 @@ AVAmbientViewingEnvironment 
> > *av_ambient_viewing_environment_create_side_data(AVF
> >   return NULL;
> >
> >   memset(side_data->data, 0, side_data->size);
> > +get_defaults((AVAmbientViewingEnvironment *)side_data->data);
> >
> >   return (AVAmbientViewingEnvironment *)side_data->data;
> >   }
>
> Will apply the set soon if nobody objectx.

In general I guess LGTM as I guess 0/0 is considered bad.

But isn't the function more about setting the defaults to a struct
rather than getting them? I just see that function being closer to
"set_defaults" than "get_defaults".

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


Re: [FFmpeg-devel] [PATCH v2 6/6] avcodec/libsvtav1: support constant quality mode

2021-09-25 Thread Jan Ekström
On Sat, Sep 25, 2021 at 4:06 AM  wrote:
>
> On Thu, Sep 23, 2021 at 09:45:48AM -0400, Christopher Degawa wrote:
> > On Sun, Sep 19, 2021 at 2:02 PM Christopher Degawa 
> > wrote:
> >
> > >
> > >
> > > On Fri, Sep 17, 2021 at 9:28 PM  wrote:
> > >
> > >> From: Limin Wang 
> > >>
> > >> Signed-off-by: Limin Wang 
> > >>
> > > As a note, I personally favor this patch over mine since I don't think
> > > `enable_tpl_la` is worth exposing to the user if its main role is to 
> > > switch
> > > between crf and cqp
> > >
> >
> > Ping on this patch, this has been requested on SVT-AV1's side as well
>
> For Jan haven't further comments, I Will apply the patchset tomorrow unless
> there are objections.

There are probably patches in this set which can be applied (like the
open/closed GOP thing, IIRC), but please do not apply the whole set.

It's kind of important we don't make this wrapper even more of a mess
than it is right now.

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


Re: [FFmpeg-devel] [PATCH v2 1/6] avcodec/libsvtav1: Fix override setting of caps_internal

2021-09-25 Thread Jan Ekström
On Sat, Sep 18, 2021 at 4:27 AM  wrote:
>
> From: Limin Wang 
>
> Signed-off-by: Limin Wang 

I'd prefer the wording "fix duplicate definition of caps_internal",
but otherwise this patch is LGTM.

Good catch.

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


Re: [FFmpeg-devel] [PATCH v2 2/6] avcodec/libsvtav1: make coded GOP type configurable

2021-09-25 Thread Jan Ekström
On Sat, Sep 18, 2021 at 4:27 AM  wrote:
>
> From: Limin Wang 
>
> Reviewed-by: Jan Ekström 
> Signed-off-by: Limin Wang 

I still hate how SVT-AV1 has no enum/defines for these, but so be it :)

LGTM.

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


Re: [FFmpeg-devel] [PATCH v2 3/6] doc: update for libsvtav1 encoder

2021-09-25 Thread Jan Ekström
On Sat, Sep 18, 2021 at 4:27 AM  wrote:
>
> From: Limin Wang 
>
> Signed-off-by: Limin Wang 
> ---

Hi,

"doc/encoders: add available values for libsvtav1 options"

would possibly be better as a commit message?

>  doc/encoders.texi | 44 
>  1 file changed, 44 insertions(+)
>
> diff --git a/doc/encoders.texi b/doc/encoders.texi
> index 8fccd73..64d604e 100644
> --- a/doc/encoders.texi
> +++ b/doc/encoders.texi
> @@ -1750,12 +1750,56 @@ You need to explicitly configure the build with 
> @code{--enable-libsvtav1}.
>  @table @option
>  @item profile
>  Set the encoding profile.
> +@table @samp
> +@item main
> +@item high
> +@item professional
> +@end table
>
>  @item level
>  Set the operating point level.
> +@table @samp
> +@item 2.0
> +@item 2.1
> +@item 2.2
> +@item 2.3
> +@item 3.0
> +@item 3.1
> +@item 3.2
> +@item 3.3
> +@item 4.0
> +@item 4.1
> +@item 4.2
> +@item 4.3
> +@item 5.0
> +@item 5.1
> +@item 5.2
> +@item 5.3
> +@item 6.0
> +@item 6.1
> +@item 6.2
> +@item 6.3
> +@item 7.0
> +@item 7.1
> +@item 7.2
> +@item 7.3
> +@end table
> +

The levels list is somewhat long, and it's specific to AV1 and not
this encoder. I don't think we list H.264/HEVC etc levels, either?

So I'm not 100% sure on this bit, possibly just noting "Set the
operating point level, f.ex. '4.0'." is good enough?

> +@item hielevel
> +Set the Hierarchical prediction levels.
> +@table @samp
> +@item 3level
> +@item 4level, This is the default.

You probably mean this to be on a separate line. Right now it ends up
as part of the entry on the list a la: ‘4level, This is the default.’

Noticed through the HTML documentation output (ffmpeg-codecs.html).

> +@end table
>
>  @item tier
>  Set the operating point tier.
> +@table @samp
> +@item main
> +The main tier was designed for most applications. This is the default.
> +@item high
> +The high tier was designed for very demanding applications.
> +@end table
>

Looking at https://aomediacodec.github.io/av1-spec/av1-spec.pdf , it
seems like the high tier only enable higher bit rates/lower
compression ratios in levels.

I think explaining it like that is probably more understandable than
vague references for "most applications" and "very demanding
applications". Or only explaining which one is the default, as I would
expect a person who needs to set this flag to know what the tier flags
in AV1 bit stream mean.

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


Re: [FFmpeg-devel] [PATCH v2 4/6] avcodec/libsvtav1: Fix CQP mode doesn't work as expection

2021-09-25 Thread Jan Ekström
On Sat, Sep 18, 2021 at 4:27 AM  wrote:
>
> From: Limin Wang 
>
> Signed-off-by: Limin Wang 
> ---

Commit message should probably be something along the lines of

"""
avcodec/libsvtav1: properly enforce CQP mode when set in wrapper

SVT-AV1 seems to have switched their default from CQP to CRF in February,
so enforce the controlling option accordingly.
"""

>  libavcodec/libsvtav1.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c
> index 0dc25ca..b029e01 100644
> --- a/libavcodec/libsvtav1.c
> +++ b/libavcodec/libsvtav1.c
> @@ -208,6 +208,8 @@ static int config_enc_params(EbSvtAv1EncConfiguration 
> *param,
>  if (param->rate_control_mode) {
>  param->max_qp_allowed   = avctx->qmax;
>  param->min_qp_allowed   = avctx->qmin;
> +} else {
> +param->enable_tpl_la = 0; /* CQP need turn off enable_tp_la */

As the default changing in the underlying library has now shown us, I
think (for now) it's better to move this next to
param->rate_control_mode earlier in the function, and then do
something like:

param->enable_tpl_la = !!param->rate_control_mode;

(I would have utilized param->enable_tpl_la = param->rate_control_mode
== SVTAV1_RC_MODE_CQP;` but alas SVT-AV1 does not have such
enums/defines that make such things more readable).

This way the parameter is set correctly no matter if the default is
switched over at SVT-AV1. In the future the wrapper can be reworked so
that by default SVT-AV1's own rate control defaults are utilized, and
then if either bit rate or cqp or something like that is set, we can
start enforcing that.

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


Re: [FFmpeg-devel] [PATCH v2 5/6] avcodec/libsvtav1: Fix the max range for rc_mode

2021-09-25 Thread Jan Ekström
On Sat, Sep 18, 2021 at 4:27 AM  wrote:
>
> From: Limin Wang 
>
> Signed-off-by: Limin Wang 
> ---

Probably something like "avcodec/libsvtav1: fix value range for
rc_mode" is a bit better as a commit message?

>  libavcodec/libsvtav1.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c
> index b029e01..509d92d 100644
> --- a/libavcodec/libsvtav1.c
> +++ b/libavcodec/libsvtav1.c
> @@ -522,7 +522,7 @@ static const AVOption options[] = {
>  #undef LEVEL
>
>  { "rc", "Bit rate control mode", OFFSET(rc_mode),
> -  AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 3, VE , "rc"},
> +  AV_OPT_TYPE_INT, { .i64 = 0 }, 0, 2, VE , "rc"},

And here we have another example of why bare numbers are bad :) .
Thanks for catching this.

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


Re: [FFmpeg-devel] [PATCH v2 6/6] avcodec/libsvtav1: support constant quality mode

2021-09-25 Thread Jan Ekström
On Thu, Sep 23, 2021 at 6:08 PM  wrote:
>
> On Thu, Sep 23, 2021 at 05:11:49PM +0300, Jan Ekström wrote:
> > On Thu, Sep 23, 2021 at 4:46 PM Christopher Degawa  
> > wrote:
> > >
> > > On Sun, Sep 19, 2021 at 2:02 PM Christopher Degawa 
> > > wrote:
> > >
> > > >
> > > >
> > > > On Fri, Sep 17, 2021 at 9:28 PM  wrote:
> > > >
> > > >> From: Limin Wang 
> > > >>
> > > >> Signed-off-by: Limin Wang 
> > > >>
> > > > As a note, I personally favor this patch over mine since I don't think
> > > > `enable_tpl_la` is worth exposing to the user if its main role is to 
> > > > switch
> > > > between crf and cqp
> > > >
> > >
> > > Ping on this patch, this has been requested on SVT-AV1's side as well
> >
> > I think my further comments on the previous version were mostly to
> > move off from the rc option for rate control, which has become more
> > and more seemingly unnecessary (and different from most other encoders
> > for no obvious reason).
> >
> > The only option that with a quick look doesn't match just setting
> > quantizer|bit rate|CRF is "cvbr", but it doesn't seem to be impossible
> > to either rework "rc" for it, or to utilize a separate option for it.
>
> Do you think it's OK to remove the rc option? to match other encoders,
> We can choose the rate control by the related parameters like:
>
> if (crf >= 0)
> choose crf rate control;
> else if (bitrate > 0)
> choose cvbr rate control;
> else if (cqp >=0 )
> choose cqp rate control;
>

In general that is the idea, yes (except bit rate would mean either
vbr or cvbr). It would be nice to follow whatever is the default on
SVT-AV1's side by default, and then if the user specifies a rate
control mode that is not the default, his selection is honored. Not
sure what the best way for that is to be honest. Possibly the style of
setting AVCodecDefault a la libx265?

For the rc option, given how much SVT-AV1 itself is in flux, I would
be OK with removing the option, or making it an option that decides
between cvbr and vbr (essentially making it "VBR bit rate control
mode"). I wish cvbr would instead be something like VBV/HRD elsewhere,
so we could just utilize maxrate&bufsize for controlling it, instead
of having it as another discrete rate control mode.

As these things change, I also hope that SVT-AV1 will soon get a
key=value style of option API, that way the wrapper will not have to
be updated constantly according to the changes in the library :) . As
I feel SVT-AV1 will be changed quite a bit in the future (due to its
recent age).

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


Re: [FFmpeg-devel] [PATCH v2 1/6] avcodec/libsvtav1: Fix override setting of caps_internal

2021-09-25 Thread Jan Ekström
On Sat, Sep 25, 2021 at 3:49 PM  wrote:
>
> On Sat, Sep 25, 2021 at 01:00:20PM +0300, Jan Ekström wrote:
> > On Sat, Sep 18, 2021 at 4:27 AM  wrote:
> > >
> > > From: Limin Wang 
> > >
> > > Signed-off-by: Limin Wang 
> >
> > I'd prefer the wording "fix duplicate definition of caps_internal",
> > but otherwise this patch is LGTM.
>
> No problem, will update with your wording.
>

For the record, those where the only comment is regarding the commit
message, I think it's at this point fair to just pull those in, so we
can make this patch set smaller on the ML :) .

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


Re: [FFmpeg-devel] [PATCH v2 6/6] avcodec/libsvtav1: support constant quality mode

2021-09-25 Thread Jan Ekström
On Sat, Sep 25, 2021 at 4:35 PM  wrote:
>
> On Sat, Sep 25, 2021 at 03:35:53PM +0300, Jan Ekström wrote:
> > On Thu, Sep 23, 2021 at 6:08 PM  wrote:
> > >
> > > On Thu, Sep 23, 2021 at 05:11:49PM +0300, Jan Ekström wrote:
> > > > On Thu, Sep 23, 2021 at 4:46 PM Christopher Degawa 
> > > >  wrote:
> > > > >
> > > > > On Sun, Sep 19, 2021 at 2:02 PM Christopher Degawa 
> > > > > 
> > > > > wrote:
> > > > >
> > > > > >
> > > > > >
> > > > > > On Fri, Sep 17, 2021 at 9:28 PM  wrote:
> > > > > >
> > > > > >> From: Limin Wang 
> > > > > >>
> > > > > >> Signed-off-by: Limin Wang 
> > > > > >>
> > > > > > As a note, I personally favor this patch over mine since I don't 
> > > > > > think
> > > > > > `enable_tpl_la` is worth exposing to the user if its main role is 
> > > > > > to switch
> > > > > > between crf and cqp
> > > > > >
> > > > >
> > > > > Ping on this patch, this has been requested on SVT-AV1's side as well
> > > >
> > > > I think my further comments on the previous version were mostly to
> > > > move off from the rc option for rate control, which has become more
> > > > and more seemingly unnecessary (and different from most other encoders
> > > > for no obvious reason).
> > > >
> > > > The only option that with a quick look doesn't match just setting
> > > > quantizer|bit rate|CRF is "cvbr", but it doesn't seem to be impossible
> > > > to either rework "rc" for it, or to utilize a separate option for it.
> > >
> > > Do you think it's OK to remove the rc option? to match other encoders,
> > > We can choose the rate control by the related parameters like:
> > >
> > > if (crf >= 0)
> > > choose crf rate control;
> > > else if (bitrate > 0)
> > > choose cvbr rate control;
> > > else if (cqp >=0 )
> > > choose cqp rate control;
> > >
> >
> > In general that is the idea, yes (except bit rate would mean either
> > vbr or cvbr). It would be nice to follow whatever is the default on
> > SVT-AV1's side by default, and then if the user specifies a rate
> > control mode that is not the default, his selection is honored. Not
> > sure what the best way for that is to be honest. Possibly the style of
> > setting AVCodecDefault a la libx265?
> >
> > For the rc option, given how much SVT-AV1 itself is in flux, I would
> > be OK with removing the option, or making it an option that decides
> > between cvbr and vbr (essentially making it "VBR bit rate control
> > mode"). I wish cvbr would instead be something like VBV/HRD elsewhere,
> > so we could just utilize maxrate&bufsize for controlling it, instead
> > of having it as another discrete rate control mode.
>
> Yes, I think it's preferable to use utilize maxrate&bufsize to control
> cbr and vbr as most of encoder. Then I'll follow this direction to update
> the patch.
>

Unfortunately that was just wishful thinking :) .

Unless you can set the buffer size and max rate in SVT-AV1, of course.
With the limited looking I've done CVBR just uses the bit rate for
each GOP instead of average over the whole clip.

> >
> > As these things change, I also hope that SVT-AV1 will soon get a
> > key=value style of option API, that way the wrapper will not have to
> > be updated constantly according to the changes in the library :) . As
> > I feel SVT-AV1 will be changed quite a bit in the future (due to its
> > recent age).
> >
>
> Yes, it's good to add svtav1-opts so that we can utilize the update options
> directly.
>

Let's go with -params ;) Since that is what modules seem to have
standardized on (x264|x265|rav1e|aom) :)

But that is anyways in the future, since SVT-AV1 doesn't have such an
API yet (as far as I know).

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] [PATCH] avformat/avio{, buf}: introduce public AVIOContext::bytes_read

2021-09-26 Thread Jan Ekström
Such a field can be seen as generally useful in cases where the
API user is not implementing custom AVIO callbacks, but still would
like to know if data is being read even if AVPackets are not being
returned.
---
Originally I thought about making an accessor for the private field, to
not grow the public struct's size (and have a duplicate field, as well
as making sure the value was read-only). But an objection was raised
that such accessors should be refrained from as they unnecessarily
filled the function symbol space or so. Together with the objection, a
proposal of making it a field on the public struct that was only written
to was proposed.

This patch follows that proposal. 

 doc/APIchanges| 3 +++
 libavformat/avio.h| 5 +
 libavformat/aviobuf.c | 2 ++
 libavformat/version.h | 2 +-
 4 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 7b267a79ac..6a8cf8ea15 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -14,6 +14,9 @@ libavutil: 2021-04-27
 
 API changes, most recent first:
 
+2021-09-26 - xx - lavf 59.6.100 - avio.h
+  Introduce a public bytes_read statistic field to AVIOContext.
+
 2021-09-21 - xx - lavu 57.7.100 - pixfmt.h
   Add AV_PIX_FMT_X2BGR10.
 
diff --git a/libavformat/avio.h b/libavformat/avio.h
index a7b56ab667..2cfb548231 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -297,6 +297,11 @@ typedef struct AVIOContext {
  * used keeping track of already written data for a later flush.
  */
 unsigned char *buf_ptr_max;
+
+/**
+ * Read-only statistic of bytes read for this AVIOContext.
+ */
+int64_t bytes_read;
 } AVIOContext;
 
 /**
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index d79e41ca77..33825ade73 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -572,6 +572,7 @@ static void fill_buffer(AVIOContext *s)
 s->buf_ptr = dst;
 s->buf_end = dst + len;
 ffiocontext(s)->bytes_read += len;
+s->bytes_read = ffiocontext(s)->bytes_read;
 }
 }
 
@@ -645,6 +646,7 @@ int avio_read(AVIOContext *s, unsigned char *buf, int size)
 } else {
 s->pos += len;
 ffiocontext(s)->bytes_read += len;
+s->bytes_read = ffiocontext(s)->bytes_read;
 size -= len;
 buf += len;
 // reset the buffer
diff --git a/libavformat/version.h b/libavformat/version.h
index 13df244d97..d5dd22059b 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  59
-#define LIBAVFORMAT_VERSION_MINOR   5
+#define LIBAVFORMAT_VERSION_MINOR   6
 #define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
-- 
2.31.1

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

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


[FFmpeg-devel] [PATCH] avformat/isom: enable TTML demuxing from MP4-likes

2021-09-26 Thread Jan Ekström
As ff_codec_movsubtitle_tags is shared between demuxing and muxing,
the muxing parts have to go in before demuxing in order to not
generate invalid media, as adding an identifier to this list enables
muxing into QTFF/MOV.
---
 libavformat/isom.c   |  2 ++
 tests/ref/fate/mov-mp4-ttml-dfxp | 10 ++
 tests/ref/fate/mov-mp4-ttml-stpp | 10 ++
 3 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/libavformat/isom.c b/libavformat/isom.c
index 4df5440023..852d237481 100644
--- a/libavformat/isom.c
+++ b/libavformat/isom.c
@@ -77,6 +77,8 @@ const AVCodecTag ff_codec_movsubtitle_tags[] = {
 { AV_CODEC_ID_MOV_TEXT, MKTAG('t', 'e', 'x', 't') },
 { AV_CODEC_ID_MOV_TEXT, MKTAG('t', 'x', '3', 'g') },
 { AV_CODEC_ID_EIA_608,  MKTAG('c', '6', '0', '8') },
+{ AV_CODEC_ID_TTML, MOV_MP4_TTML_TAG  },
+{ AV_CODEC_ID_TTML, MOV_ISMV_TTML_TAG },
 { AV_CODEC_ID_NONE, 0 },
 };
 
diff --git a/tests/ref/fate/mov-mp4-ttml-dfxp b/tests/ref/fate/mov-mp4-ttml-dfxp
index e24b5d618b..531d6856ec 100644
--- a/tests/ref/fate/mov-mp4-ttml-dfxp
+++ b/tests/ref/fate/mov-mp4-ttml-dfxp
@@ -1,13 +1,14 @@
 2e7e01c821c111466e7a2844826b7f6d *tests/data/fate/mov-mp4-ttml-dfxp.mp4
 8519 tests/data/fate/mov-mp4-ttml-dfxp.mp4
+#extradata 0:   20, 0x1dfc0302
 #tb 0: 1/1000
-#media_type 0: data
-#codec_id 0: none
+#media_type 0: subtitle
+#codec_id 0: ttml
 0,  0,  0,68500, 7866, 0x456c36b7
 {
 "packets": [
 {
-"codec_type": "data",
+"codec_type": "subtitle",
 "stream_index": 0,
 "pts": 0,
 "pts_time": "0.00",
@@ -26,7 +27,8 @@
 "streams": [
 {
 "index": 0,
-"codec_type": "data",
+"codec_name": "ttml",
+"codec_type": "subtitle",
 "codec_tag_string": "dfxp",
 "codec_tag": "0x70786664",
 "time_base": "1/1000",
diff --git a/tests/ref/fate/mov-mp4-ttml-stpp b/tests/ref/fate/mov-mp4-ttml-stpp
index 77bd23b7bf..7c03ef92cc 100644
--- a/tests/ref/fate/mov-mp4-ttml-stpp
+++ b/tests/ref/fate/mov-mp4-ttml-stpp
@@ -1,13 +1,14 @@
 cbd2c7ff864a663b0d893deac5a0caec *tests/data/fate/mov-mp4-ttml-stpp.mp4
 8547 tests/data/fate/mov-mp4-ttml-stpp.mp4
+#extradata 0:   48, 0x62100c0d
 #tb 0: 1/1000
-#media_type 0: data
-#codec_id 0: none
+#media_type 0: subtitle
+#codec_id 0: ttml
 0,  0,  0,68500, 7866, 0x456c36b7
 {
 "packets": [
 {
-"codec_type": "data",
+"codec_type": "subtitle",
 "stream_index": 0,
 "pts": 0,
 "pts_time": "0.00",
@@ -26,7 +27,8 @@ cbd2c7ff864a663b0d893deac5a0caec 
*tests/data/fate/mov-mp4-ttml-stpp.mp4
 "streams": [
 {
 "index": 0,
-"codec_type": "data",
+"codec_name": "ttml",
+"codec_type": "subtitle",
 "codec_tag_string": "stpp",
 "codec_tag": "0x70707473",
 "time_base": "1/1000",
-- 
2.31.1

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

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


Re: [FFmpeg-devel] [PATCH v3 0/5] Support for stream dispositions in MP4

2021-09-27 Thread Jan Ekström
On Mon, Sep 20, 2021 at 6:00 PM Jan Ekström  wrote:
>
> Compared to v2:
> * aviobuf changes to make a function useful in MP4 null-delimited string
>   parsing into AVBPrint.
> ** Extended read_line_to_bprint to be a more generic read_string_to_bprint.
> ** Added a maximum length argument to read_string_to_bprint.
> ** Added a new function ff_read_string_to_bprint_overwrite.
>

I would like to get more eyes on this part of the patch set, since the
C char string stuff is in my own opinion the bit where I have the
largest possibility of messing it up.

> * Switched from buffers with hard-coded sizes in avformat/mov to AVBPrint 
> based
>   parsing with ff_read_string_to_bprint_overwrite.
>
> * Minor changes to the actual primary changes of the patch set:
> ** avformat/isom: "KindWritingModeCMAF | KindWritingModeUnifiedOrigin" now
>   has spaces between the bitmasks for better readability.
> ** avformat/{isom,movenc}: add and utilize KindWritingModeNB for the maximum
>value limit in the AVOption definition.
>
> First patch implements the CMAF specified way of flagging what in FFmpeg
> are are called stream dispositions. Other identifiers such as HTML media track
> kinds are allowed, but if there is a DASH identifier for something, it should
> be utilized in stead.
>
> Second patch is a compatibility patch for one of the vendors that supports 
> this
> feature. If this is considered a too bad of a hack, we can drop it from being
> upstreamed, but at least I wanted to bring it up :) . The compatibility mode
> is not the default, so it should also not proliferate such behavior.

If there are no further comments, I will start pulling this in
tomorrow or so, with a micro version bump to note the addition of this
feature (stream dispositions) within an existing feature (movenc), as
it is API-wise not an addition of new API end points.

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] [PATCH] avformat/aacdec: enable probesize-sized resyncs mid-file

2021-09-27 Thread Jan Ekström
Before adts_aac_resync would always bail out after probesize amount
of bytes had been progressed from the start of the input.

Add an argument for the start position, and set it to zero when
reading the header (which should happen in the beginning) to mimic
previous behavior of going only up to probesize. Then, when doing
a resync mid-file when reading a packet, pass the current position
in stream to the function.

Fixes #9433
---
 libavformat/aacdec.c | 9 +
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/libavformat/aacdec.c b/libavformat/aacdec.c
index ab97be60b5..1b0e05d256 100644
--- a/libavformat/aacdec.c
+++ b/libavformat/aacdec.c
@@ -80,13 +80,14 @@ static int adts_aac_probe(const AVProbeData *p)
 return 0;
 }
 
-static int adts_aac_resync(AVFormatContext *s)
+static int adts_aac_resync(AVFormatContext *s, int64_t start_pos)
 {
 uint16_t state;
 
 // skip data until an ADTS frame is found
 state = avio_r8(s->pb);
-while (!avio_feof(s->pb) && avio_tell(s->pb) < s->probesize) {
+while (!avio_feof(s->pb) &&
+   (avio_tell(s->pb) - start_pos) < s->probesize) {
 state = (state << 8) | avio_r8(s->pb);
 if ((state >> 4) != 0xFFF)
 continue;
@@ -122,7 +123,7 @@ static int adts_aac_read_header(AVFormatContext *s)
 avio_seek(s->pb, cur, SEEK_SET);
 }
 
-ret = adts_aac_resync(s);
+ret = adts_aac_resync(s, 0);
 if (ret < 0)
 return ret;
 
@@ -187,7 +188,7 @@ retry:
 }
 if (!ff_id3v2_match(pkt->data, ID3v2_DEFAULT_MAGIC)) {
 av_packet_unref(pkt);
-ret = adts_aac_resync(s);
+ret = adts_aac_resync(s, avio_tell(s->pb));
 } else
 ret = handle_id3(s, pkt);
 if (ret < 0)
-- 
2.31.1

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

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


Re: [FFmpeg-devel] [PATCH] avformat/aacdec: enable probesize-sized resyncs mid-file

2021-09-27 Thread Jan Ekström
On Tue, Sep 28, 2021 at 1:34 AM James Almer  wrote:
>
> On 9/27/2021 6:31 PM, Jan Ekström wrote:
> > Before adts_aac_resync would always bail out after probesize amount
> > of bytes had been progressed from the start of the input.
> >
> > Add an argument for the start position, and set it to zero when
> > reading the header (which should happen in the beginning) to mimic
> > previous behavior of going only up to probesize. Then, when doing
> > a resync mid-file when reading a packet, pass the current position
> > in stream to the function.
>
> There's no need to keep the probesize limit from start of stream
> hardcoded in adts_aac_read_header(). Your solution in
> http://up-cat.net/p/e046e8f7 is IMO simpler. It will ensure any call to
> adts_aac_resync() will read only up to probesize bytes from the current
> position of the stream.
>

Alright. I thought that was done for a reason and thus tried to
emulate it, but yes - I did like my initial version's simplicity as
well.

Will post a v2 with that.

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] [PATCH v2] avformat/aacdec: enable probesize-sized resyncs mid-stream

2021-09-27 Thread Jan Ekström
Before adts_aac_resync would always bail out after probesize amount
of bytes had been progressed from the start of the input.

Now just query the current position when entering resync, and at most
advance probesize amount of data from that start position.

Fixes #9433
---
 libavformat/aacdec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/libavformat/aacdec.c b/libavformat/aacdec.c
index ab97be60b5..a476640904 100644
--- a/libavformat/aacdec.c
+++ b/libavformat/aacdec.c
@@ -83,10 +83,12 @@ static int adts_aac_probe(const AVProbeData *p)
 static int adts_aac_resync(AVFormatContext *s)
 {
 uint16_t state;
+int64_t start_pos = avio_tell(s->pb);
 
 // skip data until an ADTS frame is found
 state = avio_r8(s->pb);
-while (!avio_feof(s->pb) && avio_tell(s->pb) < s->probesize) {
+while (!avio_feof(s->pb) &&
+   (avio_tell(s->pb) - start_pos) < s->probesize) {
 state = (state << 8) | avio_r8(s->pb);
 if ((state >> 4) != 0xFFF)
 continue;
-- 
2.31.1

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

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


Re: [FFmpeg-devel] [PATCH v2] avformat/aacdec: enable probesize-sized resyncs mid-stream

2021-09-28 Thread Jan Ekström
On Tue, Sep 28, 2021 at 10:55 PM James Almer  wrote:
>
> On 9/27/2021 7:47 PM, Jan Ekström wrote:
> > Before adts_aac_resync would always bail out after probesize amount
> > of bytes had been progressed from the start of the input.
> >
> > Now just query the current position when entering resync, and at most
> > advance probesize amount of data from that start position.
> >
> > Fixes #9433
> > ---
> >   libavformat/aacdec.c | 4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavformat/aacdec.c b/libavformat/aacdec.c
> > index ab97be60b5..a476640904 100644
> > --- a/libavformat/aacdec.c
> > +++ b/libavformat/aacdec.c
> > @@ -83,10 +83,12 @@ static int adts_aac_probe(const AVProbeData *p)
> >   static int adts_aac_resync(AVFormatContext *s)
> >   {
> >   uint16_t state;
> > +int64_t start_pos = avio_tell(s->pb);
> >
> >   // skip data until an ADTS frame is found
> >   state = avio_r8(s->pb);
> > -while (!avio_feof(s->pb) && avio_tell(s->pb) < s->probesize) {
> > +while (!avio_feof(s->pb) &&
> > +   (avio_tell(s->pb) - start_pos) < s->probesize) {
> >   state = (state << 8) | avio_r8(s->pb);
> >   if ((state >> 4) != 0xFFF)
> >   continue;
>
> LGTM.

Thanks.

Applied as c20577806f0a161c6867e72f884d020a253de10a .

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


Re: [FFmpeg-devel] [PATCH] lavf/movenc: Write 'dby1' minor brand if Dolby content is being muxed to MP4

2021-09-29 Thread Jan Ekström
On Tue, Sep 28, 2021 at 5:31 PM Derek Buitenhuis
 wrote:
>
> This is as per:
>* mp4ra: http://mp4ra.org/#/brands
>* Dolby Vision muxing spec (which is public):
>
> https://professional.dolby.com/siteassets/content-creation/dolby-vision-for-content-creators/dolby_vision_bitstreams_within_the_iso_base_media_file_format_dec2017.pdf
>
> Signed-off-by: Derek Buitenhuis 
> ---
> The sole FATE change is just the brand being written for EAC-3.

I do dislike it how outside of the DoVi pdf they don't really seem to
specify 'dby1', but the mp4 registration authority's description goes
all the way back to January 2017 with this identifier
(https://github.com/mp4ra/mp4ra.github.io/blob/a27f402652b57cea190a33f6955a843869fdd457/filetype.html).

f.ex. none of the following seem to mention the 'dby1' brand:

TrueHD/MLP in MP4
https://developer.dolby.com/globalassets/technology/dolby-truehd/dolby-truehd-mlp-bitstreams-within-the-iso-base-media-file-format.pdf

(E-)AC-3 in MP4
https://www.etsi.org/deliver/etsi_ts/102300_102399/102366/01.04.01_60/ts_102366v010401p.pdf
(E-)AC-3 with Object Based Audio in MP4
https://www.etsi.org/deliver/etsi_ts/103400_103499/103420/01.02.01_60/ts_103420v010201p.pdf

AC-4 Part 1 in MP4
https://www.etsi.org/deliver/etsi_ts/103100_103199/10319001/01.03.01_60/ts_10319001v010301p.pdf
AC-4 Part 2 in MP4
https://www.etsi.org/deliver/etsi_ts/103100_103199/10319002/01.02.01_60/ts_10319002v010201p.pdf

> ---
>  libavformat/movenc.c | 9 -
>  tests/ref/fate/copy-trac3074 | 4 ++--
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/libavformat/movenc.c b/libavformat/movenc.c
> index 7650ac5ed3..fe3405d271 100644
> --- a/libavformat/movenc.c
> +++ b/libavformat/movenc.c
> @@ -4991,11 +4991,13 @@ static int mov_write_ftyp_tag(AVIOContext *pb, 
> AVFormatContext *s)
>  {
>  MOVMuxContext *mov = s->priv_data;
>  int64_t pos = avio_tell(pb);
> -int has_h264 = 0, has_av1 = 0, has_video = 0;
> +int has_h264 = 0, has_av1 = 0, has_video = 0, has_dolby = 0;
>  int i;
>
>  for (i = 0; i < s->nb_streams; i++) {
>  AVStream *st = s->streams[i];
> +AVDOVIDecoderConfigurationRecord *dovi = 
> (AVDOVIDecoderConfigurationRecord *)
> + 
> av_stream_get_side_data(st, AV_PKT_DATA_DOVI_CONF, NULL);

Maybe something a la the following to keep the line length shorter?

+AVDOVIDecoderConfigurationRecord *dovi =
+(AVDOVIDecoderConfigurationRecord *)
+av_stream_get_side_data(st, AV_PKT_DATA_DOVI_CONF, NULL);
+

>  if (is_cover_image(st))
>  continue;
>  if (st->codecpar->codec_type == AVMEDIA_TYPE_VIDEO)
> @@ -5004,6 +5006,9 @@ static int mov_write_ftyp_tag(AVIOContext *pb, 
> AVFormatContext *s)
>  has_h264 = 1;
>  if (st->codecpar->codec_id == AV_CODEC_ID_AV1)
>  has_av1 = 1;
> +if (dovi || st->codecpar->codec_id == AV_CODEC_ID_AC3 ||
> +st->codecpar->codec_id == AV_CODEC_ID_EAC3 || 
> st->codecpar->codec_id == AV_CODEC_ID_TRUEHD)
> +has_dolby = 1;

Maybe something a la the following to keep the line length shorter
(and also the codec_id checks aligned)?
+if (dovi ||
+st->codecpar->codec_id == AV_CODEC_ID_AC3 ||
+st->codecpar->codec_id == AV_CODEC_ID_EAC3 ||
+st->codecpar->codec_id == AV_CODEC_ID_TRUEHD)

(I think the next step around this code would almost be a switch/case
thing since we've got multiple of them now :D)

Otherwise LGTM.

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


Re: [FFmpeg-devel] [PATCH] avformat/avio{, buf}: introduce public AVIOContext::bytes_read

2021-10-01 Thread Jan Ekström
On Sun, Sep 26, 2021 at 6:48 PM Jan Ekström  wrote:
>
> Such a field can be seen as generally useful in cases where the
> API user is not implementing custom AVIO callbacks, but still would
> like to know if data is being read even if AVPackets are not being
> returned.
> ---

Ping.

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


Re: [FFmpeg-devel] [PATCH] avformat/avio{, buf}: introduce public AVIOContext::bytes_read

2021-10-02 Thread Jan Ekström
On Sat, Oct 2, 2021 at 1:32 PM Michael Niedermayer
 wrote:
>
> On Sun, Sep 26, 2021 at 06:48:18PM +0300, Jan Ekström wrote:
> > Such a field can be seen as generally useful in cases where the
> > API user is not implementing custom AVIO callbacks, but still would
> > like to know if data is being read even if AVPackets are not being
> > returned.
> > ---
> > Originally I thought about making an accessor for the private field, to
> > not grow the public struct's size (and have a duplicate field, as well
> > as making sure the value was read-only). But an objection was raised
> > that such accessors should be refrained from as they unnecessarily
> > filled the function symbol space or so. Together with the objection, a
> > proposal of making it a field on the public struct that was only written
> > to was proposed.
> >
> > This patch follows that proposal.
> >
> >  doc/APIchanges| 3 +++
> >  libavformat/avio.h| 5 +
> >  libavformat/aviobuf.c | 2 ++
> >  libavformat/version.h | 2 +-
> >  4 files changed, 11 insertions(+), 1 deletion(-)
>
> There are 3 statistics, read, write and seek
> shouldnt all 3 be provided to the user?
>
> thx
>

I added one which I have seen actually utilized by at least one API
client, and then others could be added as per responses.

That is why I pinged, as I had not received any responses - either
positive or negative.

Writing I can see a use for, seek I am not as sure of. But if you
believe all of them should be exposed I am fine with that.

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


Re: [FFmpeg-devel] [PATCH] avformat/avio{, buf}: introduce public AVIOContext::bytes_read

2021-10-03 Thread Jan Ekström
On Sat, Oct 2, 2021 at 2:51 PM Michael Niedermayer
 wrote:
>
> On Sat, Oct 02, 2021 at 02:42:52PM +0300, Jan Ekström wrote:
> > On Sat, Oct 2, 2021 at 1:32 PM Michael Niedermayer
> >  wrote:
> > >
> > > On Sun, Sep 26, 2021 at 06:48:18PM +0300, Jan Ekström wrote:
> > > > Such a field can be seen as generally useful in cases where the
> > > > API user is not implementing custom AVIO callbacks, but still would
> > > > like to know if data is being read even if AVPackets are not being
> > > > returned.
> > > > ---
> > > > Originally I thought about making an accessor for the private field, to
> > > > not grow the public struct's size (and have a duplicate field, as well
> > > > as making sure the value was read-only). But an objection was raised
> > > > that such accessors should be refrained from as they unnecessarily
> > > > filled the function symbol space or so. Together with the objection, a
> > > > proposal of making it a field on the public struct that was only written
> > > > to was proposed.
> > > >
> > > > This patch follows that proposal.
> > > >
> > > >  doc/APIchanges| 3 +++
> > > >  libavformat/avio.h| 5 +
> > > >  libavformat/aviobuf.c | 2 ++
> > > >  libavformat/version.h | 2 +-
> > > >  4 files changed, 11 insertions(+), 1 deletion(-)
> > >
> > > There are 3 statistics, read, write and seek
> > > shouldnt all 3 be provided to the user?
> > >
> > > thx
> > >
> >
> > I added one which I have seen actually utilized by at least one API
> > client, and then others could be added as per responses.
> >
> > That is why I pinged, as I had not received any responses - either
> > positive or negative.
> >
>
> > Writing I can see a use for, seek I am not as sure of. But if you
> > believe all of them should be exposed I am fine with that.
>
> seek is timeconsuming especially if its over a network due to
> latency.
> So for example if suddenly the number of seeks changes that
> could be interresting.
>
> thx

I would prefer to add fields which were noted as specifically private
and then cleaned up when there are actual API client users that would
see them as useful, or if there are clear use cases where they'd be
useful. I have seen the read bytes statistic actually being utilized
by an API client with a comment:

// This is fully intentional - there is no other way to get this
// information (not even by custom I/O, because the connection reuse
// mechanism by the HLS demuxer would get disabled)

(note: not sure if that any more holds true)

Also I double-checked and the write statistic was in counts, not
bytes. So that I see as generally less useful than something like
"bytes_written" if such were to exist.

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


Re: [FFmpeg-devel] [PATCH] avformat/avio{, buf}: introduce public AVIOContext::bytes_read

2021-10-03 Thread Jan Ekström
On Mon, Oct 4, 2021 at 12:25 AM Jan Ekström  wrote:
>
> On Sat, Oct 2, 2021 at 2:51 PM Michael Niedermayer
>  wrote:
> >
> > On Sat, Oct 02, 2021 at 02:42:52PM +0300, Jan Ekström wrote:
> > > On Sat, Oct 2, 2021 at 1:32 PM Michael Niedermayer
> > >  wrote:
> > > >
> > > > On Sun, Sep 26, 2021 at 06:48:18PM +0300, Jan Ekström wrote:
> > > > > Such a field can be seen as generally useful in cases where the
> > > > > API user is not implementing custom AVIO callbacks, but still would
> > > > > like to know if data is being read even if AVPackets are not being
> > > > > returned.
> > > > > ---
> > > > > Originally I thought about making an accessor for the private field, 
> > > > > to
> > > > > not grow the public struct's size (and have a duplicate field, as well
> > > > > as making sure the value was read-only). But an objection was raised
> > > > > that such accessors should be refrained from as they unnecessarily
> > > > > filled the function symbol space or so. Together with the objection, a
> > > > > proposal of making it a field on the public struct that was only 
> > > > > written
> > > > > to was proposed.
> > > > >
> > > > > This patch follows that proposal.
> > > > >
> > > > >  doc/APIchanges| 3 +++
> > > > >  libavformat/avio.h| 5 +
> > > > >  libavformat/aviobuf.c | 2 ++
> > > > >  libavformat/version.h | 2 +-
> > > > >  4 files changed, 11 insertions(+), 1 deletion(-)
> > > >
> > > > There are 3 statistics, read, write and seek
> > > > shouldnt all 3 be provided to the user?
> > > >
> > > > thx
> > > >
> > >
> > > I added one which I have seen actually utilized by at least one API
> > > client, and then others could be added as per responses.
> > >
> > > That is why I pinged, as I had not received any responses - either
> > > positive or negative.
> > >
> >
> > > Writing I can see a use for, seek I am not as sure of. But if you
> > > believe all of them should be exposed I am fine with that.
> >
> > seek is timeconsuming especially if its over a network due to
> > latency.
> > So for example if suddenly the number of seeks changes that
> > could be interresting.
> >
> > thx
>
> I would prefer to add fields which were noted as specifically private
> and then cleaned up when there are actual API client users that would
> see them as useful, or if there are clear use cases where they'd be
> useful. I have seen the read bytes statistic actually being utilized
> by an API client with a comment:
>
> // This is fully intentional - there is no other way to get this
> // information (not even by custom I/O, because the connection reuse
> // mechanism by the HLS demuxer would get disabled)
>
> (note: not sure if that any more holds true)
>
> Also I double-checked and the write statistic was in counts, not
> bytes. So that I see as generally less useful than something like
> "bytes_written" if such were to exist.

Also just checked, and AVIOContext::written seems to be this
"bytes_written" already :) Just completely undocumented.

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


Re: [FFmpeg-devel] [PATCH] avformat/avio{, buf}: introduce public AVIOContext::bytes_read

2021-10-04 Thread Jan Ekström
On Mon, Oct 4, 2021 at 1:06 AM Michael Niedermayer
 wrote:
>
> On Mon, Oct 04, 2021 at 12:25:26AM +0300, Jan Ekström wrote:
> > On Sat, Oct 2, 2021 at 2:51 PM Michael Niedermayer
> >  wrote:
> > >
> > > On Sat, Oct 02, 2021 at 02:42:52PM +0300, Jan Ekström wrote:
> > > > On Sat, Oct 2, 2021 at 1:32 PM Michael Niedermayer
> > > >  wrote:
> > > > >
> > > > > On Sun, Sep 26, 2021 at 06:48:18PM +0300, Jan Ekström wrote:
> > > > > > Such a field can be seen as generally useful in cases where the
> > > > > > API user is not implementing custom AVIO callbacks, but still would
> > > > > > like to know if data is being read even if AVPackets are not being
> > > > > > returned.
> > > > > > ---
> > > > > > Originally I thought about making an accessor for the private 
> > > > > > field, to
> > > > > > not grow the public struct's size (and have a duplicate field, as 
> > > > > > well
> > > > > > as making sure the value was read-only). But an objection was raised
> > > > > > that such accessors should be refrained from as they unnecessarily
> > > > > > filled the function symbol space or so. Together with the 
> > > > > > objection, a
> > > > > > proposal of making it a field on the public struct that was only 
> > > > > > written
> > > > > > to was proposed.
> > > > > >
> > > > > > This patch follows that proposal.
> > > > > >
> > > > > >  doc/APIchanges| 3 +++
> > > > > >  libavformat/avio.h| 5 +
> > > > > >  libavformat/aviobuf.c | 2 ++
> > > > > >  libavformat/version.h | 2 +-
> > > > > >  4 files changed, 11 insertions(+), 1 deletion(-)
> > > > >
> > > > > There are 3 statistics, read, write and seek
> > > > > shouldnt all 3 be provided to the user?
> > > > >
> > > > > thx
> > > > >
> > > >
> > > > I added one which I have seen actually utilized by at least one API
> > > > client, and then others could be added as per responses.
> > > >
> > > > That is why I pinged, as I had not received any responses - either
> > > > positive or negative.
> > > >
> > >
> > > > Writing I can see a use for, seek I am not as sure of. But if you
> > > > believe all of them should be exposed I am fine with that.
> > >
> > > seek is timeconsuming especially if its over a network due to
> > > latency.
> > > So for example if suddenly the number of seeks changes that
> > > could be interresting.
> > >
> > > thx
> >
> > I would prefer to add fields which were noted as specifically private
> > and then cleaned up when there are actual API client users that would
> > see them as useful, or if there are clear use cases where they'd be
> > useful. I have seen the read bytes statistic actually being utilized
> > by an API client with a comment:
>
> Assume a network protocol, TCP, UDP, HTTP, RT*P whatever
> how do you tune the buffer sizes ?
> Can the number of seeks be used ?
> or from a different point of view, if there are alot of seeks should
> a user app try to increase the buffer sizes ?
>
> maybe iam missing something but when playing a not perfectly interleaved file
> over the network the buffer size should be what makes the difference between
> that working or not working
> ideally a user app shouldnt need to mess with this, of course and these values
> should all be automagically adjusted
>
> If a user app fails to get packets in realtime over the network, it would
> fail to play that stream. Some user apps could display a warning message to
> the user about it.
> If now the user app has access to the number of seeks it could be more
> specific in the warning to the user.
> "Unable to play network is maybe too slow"
> "Unable to play buffer is maybe too small or file is poorly interleaved"
> ...
>
> Maybe iam just seeing all this from the wrong side i dunno but to me it seems
> usefull to a user app to have access to the number of seeks and these seem
> non contrived use cases to me ... Ive gotten random point to nowhere
> warnings about playback issues and restarting the computer obviously that
> never was the issue.
>
> thx
>

OK, I think this is now focusing on the wrong point, sorry.

I think it's just better for me to note that I am not the best person
to post (and thus be the one to argue for the usefulness in reviews if
someone asks why I am bringing those private entries that were once
removed back to the public struct) of those other entries.

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


Re: [FFmpeg-devel] [PATCH v3 0/5] Support for stream dispositions in MP4

2021-10-04 Thread Jan Ekström
On Mon, Sep 27, 2021 at 1:53 PM Jan Ekström  wrote:
>
> On Mon, Sep 20, 2021 at 6:00 PM Jan Ekström  wrote:
> >
> > Compared to v2:
> > * aviobuf changes to make a function useful in MP4 null-delimited string
> >   parsing into AVBPrint.
> > ** Extended read_line_to_bprint to be a more generic read_string_to_bprint.
> > ** Added a maximum length argument to read_string_to_bprint.
> > ** Added a new function ff_read_string_to_bprint_overwrite.
> >
>
> I would like to get more eyes on this part of the patch set, since the
> C char string stuff is in my own opinion the bit where I have the
> largest possibility of messing it up.
>
> > * Switched from buffers with hard-coded sizes in avformat/mov to AVBPrint 
> > based
> >   parsing with ff_read_string_to_bprint_overwrite.
> >
> > * Minor changes to the actual primary changes of the patch set:
> > ** avformat/isom: "KindWritingModeCMAF | KindWritingModeUnifiedOrigin" now
> >   has spaces between the bitmasks for better readability.
> > ** avformat/{isom,movenc}: add and utilize KindWritingModeNB for the maximum
> >value limit in the AVOption definition.
> >
> > First patch implements the CMAF specified way of flagging what in FFmpeg
> > are are called stream dispositions. Other identifiers such as HTML media 
> > track
> > kinds are allowed, but if there is a DASH identifier for something, it 
> > should
> > be utilized in stead.
> >
> > Second patch is a compatibility patch for one of the vendors that supports 
> > this
> > feature. If this is considered a too bad of a hack, we can drop it from 
> > being
> > upstreamed, but at least I wanted to bring it up :) . The compatibility mode
> > is not the default, so it should also not proliferate such behavior.
>
> If there are no further comments, I will start pulling this in
> tomorrow or so, with a micro version bump to note the addition of this
> feature (stream dispositions) within an existing feature (movenc), as
> it is API-wise not an addition of new API end points.
>

Since there were no further comments, I:

1. updated the test result to match the newly added dby1 compatible brand
2. bumped the lavf micro version since while this doesn't add new
codecs or (de)muxers, it is a new capability
3. applied everything but the second mp4-related patch of the set,
adding compatibility against a specific vendor in order to hopefully
encourage them to follow the specification.

Hashes 94f227bac1c0189d5a270322398bfa4ffa6ad196
,151f46e84ddce557aace102a9f86f72d37e1cdbf ,
847fd8de7c13abe41ca59464014f17c56555ef7b and
7a446b1179301b6b9d05a7d39574e75e8fa5a862 .

Thanks to Martin for having a discussion on the last patch in the set,
and for everyone else who took a look at the patch set otherwise.

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


Re: [FFmpeg-devel] point releases

2021-10-07 Thread Jan Ekström
On Thu, Oct 7, 2021 at 11:09 AM Steven Liu  wrote:
>
> Steven Liu  于2021年10月7日周四 上午10:41写道:
> >
> > Michael Niedermayer  于2021年10月6日周三 下午5:35写道:
> > >
> > > Hi all
> > >
> > > I do plan to make releases from
> > > 4.4, 4.2, 4.1, 4.0, 3.4, 3.2, 2.8 branches
> > Why not include 4.3 or have some reason?
> Ok, i get it, because it have not present in downstreams page :D
> >
> >
> >
> > Thanks
> > Steven

FYI someone popped up on IRC earlier requesting that
https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/59032494e81a1a65c0b960aaae7ec4c2cc9db35a
be back-ported to release/4.4 . you should evaluate this request and
see whether it makes sense.

Best regards,
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".


Re: [FFmpeg-devel] point releases

2021-10-07 Thread Jan Ekström
On Thu, Oct 7, 2021 at 12:13 PM Jan Ekström  wrote:
>
> On Thu, Oct 7, 2021 at 11:09 AM Steven Liu  wrote:
> >
> > Steven Liu  于2021年10月7日周四 上午10:41写道:
> > >
> > > Michael Niedermayer  于2021年10月6日周三 下午5:35写道:
> > > >
> > > > Hi all
> > > >
> > > > I do plan to make releases from
> > > > 4.4, 4.2, 4.1, 4.0, 3.4, 3.2, 2.8 branches
> > > Why not include 4.3 or have some reason?
> > Ok, i get it, because it have not present in downstreams page :D
> > >
> > >
> > >
> > > Thanks
> > > Steven
>
> FYI someone popped up on IRC earlier requesting that
> https://git.ffmpeg.org/gitweb/ffmpeg.git/commit/59032494e81a1a65c0b960aaae7ec4c2cc9db35a
> be back-ported to release/4.4 . you should evaluate this request and
> see whether it makes sense.

As an additional note, it does seem to apply cleanly with `git
cherry-pick -x 59032494e81a1a65c0b960aaae7ec4c2cc9db35a`

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


Re: [FFmpeg-devel] [PATCH] avformat/avio{, buf}: introduce public AVIOContext::bytes_read

2021-10-11 Thread Jan Ekström
On Mon, Oct 4, 2021 at 12:12 PM Jan Ekström  wrote:
>
> On Mon, Oct 4, 2021 at 1:06 AM Michael Niedermayer
>  wrote:
> >
> > On Mon, Oct 04, 2021 at 12:25:26AM +0300, Jan Ekström wrote:
> > > On Sat, Oct 2, 2021 at 2:51 PM Michael Niedermayer
> > >  wrote:
> > > >
> > > > On Sat, Oct 02, 2021 at 02:42:52PM +0300, Jan Ekström wrote:
> > > > > On Sat, Oct 2, 2021 at 1:32 PM Michael Niedermayer
> > > > >  wrote:
> > > > > >
> > > > > > On Sun, Sep 26, 2021 at 06:48:18PM +0300, Jan Ekström wrote:
> > > > > > > Such a field can be seen as generally useful in cases where the
> > > > > > > API user is not implementing custom AVIO callbacks, but still 
> > > > > > > would
> > > > > > > like to know if data is being read even if AVPackets are not being
> > > > > > > returned.
> > > > > > > ---
> > > > > > > Originally I thought about making an accessor for the private 
> > > > > > > field, to
> > > > > > > not grow the public struct's size (and have a duplicate field, as 
> > > > > > > well
> > > > > > > as making sure the value was read-only). But an objection was 
> > > > > > > raised
> > > > > > > that such accessors should be refrained from as they unnecessarily
> > > > > > > filled the function symbol space or so. Together with the 
> > > > > > > objection, a
> > > > > > > proposal of making it a field on the public struct that was only 
> > > > > > > written
> > > > > > > to was proposed.
> > > > > > >
> > > > > > > This patch follows that proposal.
> > > > > > >
> > > > > > >  doc/APIchanges| 3 +++
> > > > > > >  libavformat/avio.h| 5 +
> > > > > > >  libavformat/aviobuf.c | 2 ++
> > > > > > >  libavformat/version.h | 2 +-
> > > > > > >  4 files changed, 11 insertions(+), 1 deletion(-)
> > > > > >
> > > > > > There are 3 statistics, read, write and seek
> > > > > > shouldnt all 3 be provided to the user?
> > > > > >
> > > > > > thx
> > > > > >
> > > > >
> > > > > I added one which I have seen actually utilized by at least one API
> > > > > client, and then others could be added as per responses.
> > > > >
> > > > > That is why I pinged, as I had not received any responses - either
> > > > > positive or negative.
> > > > >
> > > >
> > > > > Writing I can see a use for, seek I am not as sure of. But if you
> > > > > believe all of them should be exposed I am fine with that.
> > > >
> > > > seek is timeconsuming especially if its over a network due to
> > > > latency.
> > > > So for example if suddenly the number of seeks changes that
> > > > could be interresting.
> > > >
> > > > thx
> > >
> > > I would prefer to add fields which were noted as specifically private
> > > and then cleaned up when there are actual API client users that would
> > > see them as useful, or if there are clear use cases where they'd be
> > > useful. I have seen the read bytes statistic actually being utilized
> > > by an API client with a comment:
> >
> > Assume a network protocol, TCP, UDP, HTTP, RT*P whatever
> > how do you tune the buffer sizes ?
> > Can the number of seeks be used ?
> > or from a different point of view, if there are alot of seeks should
> > a user app try to increase the buffer sizes ?
> >
> > maybe iam missing something but when playing a not perfectly interleaved 
> > file
> > over the network the buffer size should be what makes the difference between
> > that working or not working
> > ideally a user app shouldnt need to mess with this, of course and these 
> > values
> > should all be automagically adjusted
> >
> > If a user app fails to get packets in realtime over the network, it would
> > fail to play that stream. Some user apps could display a warning message to
> > the user about it.
> > If now the user app has access to the number of seeks it could be more
> > specific in the warning to the user.
> > "Unable to play network is maybe too slow"
> > "Unable to play buffer is maybe too small or file is poorly interleaved"
> > ...
> >
> > Maybe iam just seeing all this from the wrong side i dunno but to me it 
> > seems
> > usefull to a user app to have access to the number of seeks and these seem
> > non contrived use cases to me ... Ive gotten random point to nowhere
> > warnings about playback issues and restarting the computer obviously that
> > never was the issue.
> >
> > thx
> >
>
> OK, I think this is now focusing on the wrong point, sorry.
>
> I think it's just better for me to note that I am not the best person
> to post (and thus be the one to argue for the usefulness in reviews if
> someone asks why I am bringing those private entries that were once
> removed back to the public struct) of those other entries.

Ping? Is this now in a purgatory state due to me not wanting to be the
one to argue for the other options in review?

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


Re: [FFmpeg-devel] [PATCH 1/1] mov: read track title

2021-10-11 Thread Jan Ekström
On Mon, Oct 11, 2021 at 6:23 AM Dong Nguyen  wrote:
>
> this change fix issue [9438](https://trac.ffmpeg.org/ticket/9438)
>
> after commit da9cc22d5bd5f59756c2037b02966376da2cf323
> ffmpeg is able to write track title metadata to mov/mp4 format file
> but it is not able to read back the metadata
>

Huh, so I seem to have incorrectly read things previously :) .I
started implementing reading of the 3GPP titl box due to the QTFF spec
saying that `name` is not supposed to be an end user facing name, but
an internal/debug reference [1].

Of course, then checking the actual contents of the 'name' box within
the udta box, it seems like I was incorrect, as the box's structure
doesn't match. The udta side of the spec has a one-liner that doesn't
say much about how it should be utilized [2].

So I guess this udta `name` is in this case not something internal,
after all? ┐(´д`)┌

[1] 
https://developer.apple.com/library/archive/documentation/QuickTime/QTFF/Metadata/Metadata.html
[2] 
https://developer.apple.com/library/archive/documentation/QuickTime/QTFF/QTFFChap2/qtff2.html#//apple_ref/doc/uid/TP4939-CH204-25538

> Signed-off-by: Dong Nguyen 
> ---
>  libavformat/mov.c | 15 +--
>  1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index a811bc7677..644e746d02 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -303,6 +303,7 @@ static int mov_read_udta_string(MOVContext *c, 
> AVIOContext *pb, MOVAtom atom)
>  int (*parse)(MOVContext*, AVIOContext*, unsigned, const char*) = NULL;
>  int raw = 0;
>  int num = 0;
> +int track_title = 0;
>
>  switch (atom.type) {
>  case MKTAG( '@','P','R','M'): key = "premiere_version"; raw = 1; break;
> @@ -380,6 +381,12 @@ static int mov_read_udta_string(MOVContext *c, 
> AVIOContext *pb, MOVAtom atom)
>  case MKTAG(0xa9,'l','y','r'): key = "lyrics";break;
>  case MKTAG(0xa9,'m','a','k'): key = "make";  break;
>  case MKTAG(0xa9,'m','o','d'): key = "model"; break;
> +case MKTAG('n','a','m','e') :
> +if (c->trak_index >= 0) { // meta inside track
> +key = "title";
> +track_title = 1;
> +}
> +break;

Since this seems to be a barebones string-only box, if you set `key =
"title"; raw = 1;` the string parsing should work. No need to add the
track_title variable to skip other types of string parsing (data_type
is zero so it shouldn't hit any of those pieces of custom data parsing
logic, either).

>  case MKTAG(0xa9,'n','a','m'): key = "title"; break;
>  case MKTAG(0xa9,'o','p','e'): key = "original_artist"; break;
>  case MKTAG(0xa9,'p','r','d'): key = "producer";  break;
> @@ -428,7 +435,7 @@ retry:
>  return ret;
>  }
>  } else return 0;
> -} else if (atom.size > 4 && key && !c->itunes_metadata && !raw) {
> +} else if (atom.size > 4 && key && !c->itunes_metadata && !raw && 
> !track_title) {
>  str_size = avio_rb16(pb); // string length
>  if (str_size > atom.size) {
>  raw = 1;
> @@ -521,7 +528,11 @@ retry:
>  str[str_size] = 0;
>  }
>  c->fc->event_flags |= AVFMT_EVENT_FLAG_METADATA_UPDATED;
> -av_dict_set(&c->fc->metadata, key, str, 0);
> +if (track_title) {
> +av_dict_set(&c->fc->streams[c->trak_index]->metadata, key, str, 
> 0);
> +} else {
> +av_dict_set(&c->fc->metadata, key, str, 0);
> +}

Since QTFF, ISOBMFF, 3GPP and such all seem to utilize udta in both
movie and track boxes, I think the correct thing to do would be to
first to poke in something a la
https://github.com/jeeb/ffmpeg/commits/enable_writing_udta_metadata_for_tracks
.

Unfortunately currently movenc's track udta box writing seems to be
*very* limited, which thus leads to things which were read from track
udta boxes to not be propagated through (and thus you get a lot of
FATE test diffs where information is only seemingly removed and not
propagated from another location). Welcome to the mess that is
mov(enc) :) .

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] [PATCH v2 0/3] introduce public AVIOContext::bytes_{read, written}

2021-10-13 Thread Jan Ekström
After a brief discussion with Michael on IRC, this seems to be the idea of a
path forward that was agreed upon.

1. AVIOContext::written was supposed to be a private field [1], so move the
   value utilized internally to FFIOContext, and set the AVIOContext value
   from there.
2. Introduce public AVIOContext::bytes_{read,written} statistics fields.
3. Deprecate AVIOContext::written.

I was not sure whether deprecation or straight-out removal was the right thing
to do - since while the field was meant to be internal it was not marked as
such in FFmpeg's merged state of the struct (which is why it did not get
cleaned up into FFIOContext earlier) - but in order to not get stuck on that,
I am now posting this with the full deprecation changes. This way (hopefully)
this change will get more eyeballs and if someone thinks this could just be
removed the patches can be changed to do that instead.

[1] 
http://git.videolan.org/?p=ffmpeg.git;a=commit;h=3f75e5116b900f1428aa13041fc7d6301bf1988a

Best regards,
Jan

Jan Ekström (3):
  avformat/avio: privatize point of truth for AVIOContext::written
  avformat/avio{,buf}: introduce public
AVIOContext::bytes_{read,written}
  avformat/avio{,buf}: deprecate AVIOContext::written

 doc/APIchanges  | 12 
 libavformat/avio.h  | 16 
 libavformat/avio_internal.h |  5 +
 libavformat/aviobuf.c   | 22 ++
 libavformat/version.h   |  5 -
 5 files changed, 55 insertions(+), 5 deletions(-)

-- 
2.31.1

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

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


[FFmpeg-devel] [PATCH v2 1/3] avformat/avio: privatize point of truth for AVIOContext::written

2021-10-13 Thread Jan Ekström
Looking at 3f75e5116b900f1428aa13041fc7d6301bf1988a, the field
was supposed to be private, but during merging the field and the
group that had the comment about it got separated.

Thus, move the actual privately utilized state of this variable
into the private FFIOContext.
---
 libavformat/avio_internal.h |  5 +
 libavformat/aviobuf.c   | 11 +++
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/libavformat/avio_internal.h b/libavformat/avio_internal.h
index eded38759b..28ea38079d 100644
--- a/libavformat/avio_internal.h
+++ b/libavformat/avio_internal.h
@@ -51,6 +51,11 @@ typedef struct FFIOContext {
  */
 int64_t bytes_read;
 
+/**
+ * Bytes written statistic
+ */
+int64_t bytes_written;
+
 /**
  * seek statistic
  */
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 3d87d66091..7afbff0266 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -164,8 +164,10 @@ static void writeout(AVIOContext *s, const uint8_t *data, 
int len)
 if (ret < 0) {
 s->error = ret;
 } else {
-if (s->pos + len > s->written)
-s->written = s->pos + len;
+if (s->pos + len > ctx->bytes_written) {
+ctx->bytes_written = s->pos + len;
+s->written = ctx->bytes_written;
+}
 }
 }
 if (ctx->current_type == AVIO_DATA_MARKER_SYNC_POINT ||
@@ -337,13 +339,14 @@ int64_t avio_skip(AVIOContext *s, int64_t offset)
 
 int64_t avio_size(AVIOContext *s)
 {
+FFIOContext *const ctx = ffiocontext(s);
 int64_t size;
 
 if (!s)
 return AVERROR(EINVAL);
 
-if (s->written)
-return s->written;
+if (ctx->bytes_written)
+return ctx->bytes_written;
 
 if (!s->seek)
 return AVERROR(ENOSYS);
-- 
2.31.1

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

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


[FFmpeg-devel] [PATCH v2 2/3] avformat/avio{, buf}: introduce public AVIOContext::bytes_{read, written}

2021-10-13 Thread Jan Ekström
Such fields can be seen as generally useful in cases where the
API user is not implementing custom AVIO callbacks, but still would
like to know if data is being read or written out, such as in case
data is being read from input but no AVPacket has been received yet.
---
 doc/APIchanges|  3 +++
 libavformat/avio.h| 10 ++
 libavformat/aviobuf.c |  4 +++-
 libavformat/version.h |  2 +-
 4 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 7b267a79ac..806eaf4c83 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -14,6 +14,9 @@ libavutil: 2021-04-27
 
 API changes, most recent first:
 
+2021-09-26 - xx - lavf 59.7.100 - avio.h
+  Introduce public bytes_{read,written} statistic fields to AVIOContext.
+
 2021-09-21 - xx - lavu 57.7.100 - pixfmt.h
   Add AV_PIX_FMT_X2BGR10.
 
diff --git a/libavformat/avio.h b/libavformat/avio.h
index a7b56ab667..0f9a0f909f 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -297,6 +297,16 @@ typedef struct AVIOContext {
  * used keeping track of already written data for a later flush.
  */
 unsigned char *buf_ptr_max;
+
+/**
+ * Read-only statistic of bytes read for this AVIOContext.
+ */
+int64_t bytes_read;
+
+/**
+ * Read-only statistic of bytes written for this AVIOContext.
+ */
+int64_t bytes_written;
 } AVIOContext;
 
 /**
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 7afbff0266..436a264f83 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -165,7 +165,7 @@ static void writeout(AVIOContext *s, const uint8_t *data, 
int len)
 s->error = ret;
 } else {
 if (s->pos + len > ctx->bytes_written) {
-ctx->bytes_written = s->pos + len;
+s->bytes_written = ctx->bytes_written = s->pos + len;
 s->written = ctx->bytes_written;
 }
 }
@@ -575,6 +575,7 @@ static void fill_buffer(AVIOContext *s)
 s->buf_ptr = dst;
 s->buf_end = dst + len;
 ffiocontext(s)->bytes_read += len;
+s->bytes_read = ffiocontext(s)->bytes_read;
 }
 }
 
@@ -648,6 +649,7 @@ int avio_read(AVIOContext *s, unsigned char *buf, int size)
 } else {
 s->pos += len;
 ffiocontext(s)->bytes_read += len;
+s->bytes_read = ffiocontext(s)->bytes_read;
 size -= len;
 buf += len;
 // reset the buffer
diff --git a/libavformat/version.h b/libavformat/version.h
index d5dd22059b..474640bb78 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  59
-#define LIBAVFORMAT_VERSION_MINOR   6
+#define LIBAVFORMAT_VERSION_MINOR   7
 #define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
-- 
2.31.1

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

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


[FFmpeg-devel] [PATCH v2 3/3] avformat/avio{, buf}: deprecate AVIOContext::written

2021-10-13 Thread Jan Ekström
Originally added as a private entry in commit
3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
the comment noting its private state was missed during merging of
the field from Libav (most likely due to an already existing field
in between).

Users should migrate to the public field AVIOContext::bytes_written.
---
 doc/APIchanges| 9 +
 libavformat/avio.h| 6 ++
 libavformat/aviobuf.c | 9 +
 libavformat/version.h | 5 -
 4 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 806eaf4c83..2fc964d80d 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -14,6 +14,15 @@ libavutil: 2021-04-27
 
 API changes, most recent first:
 
+2021-10-13 - xx - lavf 59.8.100 - avio.h
+  Deprecate AVIOContext.written. Originally added as a private entry in
+  commit 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
+  the comment noting its private state was missed during merging of the field
+  from Libav (most likely due to an already existing field in between).
+
+  Users should migrate to the public field AVIOContext.bytes_written, which
+  returns the same value.
+
 2021-09-26 - xx - lavf 59.7.100 - avio.h
   Introduce public bytes_{read,written} statistic fields to AVIOContext.
 
diff --git a/libavformat/avio.h b/libavformat/avio.h
index 0f9a0f909f..7d33e971cb 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -290,7 +290,13 @@ typedef struct AVIOContext {
  */
 int ignore_boundary_point;
 
+#if FF_API_AVIOCONTEXT_WRITTEN
+/**
+ * @deprecated AVIOContext::bytes_written should be utilized instead.
+ */
+attribute_deprecated
 int64_t written;
+#endif
 
 /**
  * Maximum reached position before a backward seek in the write buffer,
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 436a264f83..7a0cd27151 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -22,6 +22,7 @@
 #include "libavutil/bprint.h"
 #include "libavutil/crc.h"
 #include "libavutil/dict.h"
+#include "libavutil/internal.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/log.h"
 #include "libavutil/opt.h"
@@ -124,7 +125,11 @@ void ffio_init_context(FFIOContext *ctx,
 ctx->current_type= AVIO_DATA_MARKER_UNKNOWN;
 ctx->last_time   = AV_NOPTS_VALUE;
 ctx->short_seek_get  = NULL;
+#if FF_API_AVIOCONTEXT_WRITTEN
+FF_DISABLE_DEPRECATION_WARNINGS
 s->written   = 0;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
 }
 
 AVIOContext *avio_alloc_context(
@@ -166,7 +171,11 @@ static void writeout(AVIOContext *s, const uint8_t *data, 
int len)
 } else {
 if (s->pos + len > ctx->bytes_written) {
 s->bytes_written = ctx->bytes_written = s->pos + len;
+#if FF_API_AVIOCONTEXT_WRITTEN
+FF_DISABLE_DEPRECATION_WARNINGS
 s->written = ctx->bytes_written;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
 }
 }
 }
diff --git a/libavformat/version.h b/libavformat/version.h
index 474640bb78..81ed517609 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  59
-#define LIBAVFORMAT_VERSION_MINOR   7
+#define LIBAVFORMAT_VERSION_MINOR   8
 #define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
@@ -61,6 +61,9 @@
 #ifndef FF_API_COMPUTE_PKT_FIELDS2
 #define FF_API_COMPUTE_PKT_FIELDS2  (LIBAVFORMAT_VERSION_MAJOR < 60)
 #endif
+#ifndef FF_API_AVIOCONTEXT_WRITTEN
+#define FF_API_AVIOCONTEXT_WRITTEN  (LIBAVFORMAT_VERSION_MAJOR < 60)
+#endif
 
 
 #ifndef FF_API_R_FRAME_RATE
-- 
2.31.1

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

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


[FFmpeg-devel] [PATCH v3 0/3] introduce public AVIOContext::bytes_{read, written}

2021-10-18 Thread Jan Ekström
Changes compared to v2:
* Written was actually written_size, so it did not take into account any
  writes after a seek-back. Thus an initial attempt at implementing
  bytes_written was made.

After a brief discussion with Michael on IRC, this seems to be the idea of a
path forward that was agreed upon.

1. AVIOContext::written was supposed to be a private field [1], so move the
   value utilized internally to FFIOContext, and set the AVIOContext value
   from there.
2. Deprecate AVIOContext::written.
3. Introduce public AVIOContext::bytes_{read,written} statistics fields.

I was not sure whether deprecation or straight-out removal was the right thing
to do - since while the field was meant to be internal it was not marked as
such in FFmpeg's merged state of the struct (which is why it did not get
cleaned up into FFIOContext earlier) - but in order to not get stuck on that,
I am now posting this with the full deprecation changes. This way (hopefully)
this change will get more eyeballs and if someone thinks this could just be
removed the patches can be changed to do that instead.

[1] 
http://git.videolan.org/?p=ffmpeg.git;a=commit;h=3f75e5116b900f1428aa13041fc7d6301bf1988a

Best regards,
Jan

Jan Ekström (3):
  avformat/avio: privatize point of truth for AVIOContext::written
  avformat/avio{,buf}: deprecate AVIOContext::written
  avformat/avio{,buf}: introduce public
AVIOContext::bytes_{read,written}

 doc/APIchanges  |  9 +
 libavformat/avio.h  | 18 ++
 libavformat/avio_internal.h | 11 +++
 libavformat/aviobuf.c   | 30 --
 libavformat/version.h   |  5 -
 5 files changed, 66 insertions(+), 7 deletions(-)

-- 
2.31.1

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

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


[FFmpeg-devel] [PATCH v3 1/3] avformat/avio: privatize point of truth for AVIOContext::written

2021-10-18 Thread Jan Ekström
Looking at 3f75e5116b900f1428aa13041fc7d6301bf1988a, the field
was supposed to be private, but during merging the field and the
group that had the comment about it got separated.

Thus, move the actual privately utilized state of this variable
into the private FFIOContext. Additionally, name the private field
somewhat better, so that it does not get confused with the amount
of bytes written out.
---
 libavformat/avio_internal.h |  6 ++
 libavformat/aviobuf.c   | 11 +++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/libavformat/avio_internal.h b/libavformat/avio_internal.h
index eded38759b..467e80701f 100644
--- a/libavformat/avio_internal.h
+++ b/libavformat/avio_internal.h
@@ -66,6 +66,12 @@ typedef struct FFIOContext {
  * used after probing to ensure seekback and to reset the buffer size
  */
 int orig_buffer_size;
+
+/**
+ * Written output size
+ * is updated each time a successful writeout ends up further position-wise
+ */
+int64_t written_output_size;
 } FFIOContext;
 
 static av_always_inline FFIOContext *ffiocontext(AVIOContext *ctx)
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index 3d87d66091..b18a56ef19 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -164,8 +164,10 @@ static void writeout(AVIOContext *s, const uint8_t *data, 
int len)
 if (ret < 0) {
 s->error = ret;
 } else {
-if (s->pos + len > s->written)
-s->written = s->pos + len;
+if (s->pos + len > ctx->written_output_size) {
+ctx->written_output_size = s->pos + len;
+s->written = ctx->written_output_size;
+}
 }
 }
 if (ctx->current_type == AVIO_DATA_MARKER_SYNC_POINT ||
@@ -337,13 +339,14 @@ int64_t avio_skip(AVIOContext *s, int64_t offset)
 
 int64_t avio_size(AVIOContext *s)
 {
+FFIOContext *const ctx = ffiocontext(s);
 int64_t size;
 
 if (!s)
 return AVERROR(EINVAL);
 
-if (s->written)
-return s->written;
+if (ctx->written_output_size)
+return ctx->written_output_size;
 
 if (!s->seek)
 return AVERROR(ENOSYS);
-- 
2.31.1

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

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


[FFmpeg-devel] [PATCH v3 2/3] avformat/avio{, buf}: deprecate AVIOContext::written

2021-10-18 Thread Jan Ekström
Originally added as a private entry in commit
3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
the comment noting its private state was missed during merging of
the field from Libav (most likely due to an already existing field
in between).
---
 doc/APIchanges| 6 ++
 libavformat/avio.h| 6 ++
 libavformat/aviobuf.c | 9 +
 libavformat/version.h | 5 -
 4 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 7b267a79ac..4731e14cb1 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -14,6 +14,12 @@ libavutil: 2021-04-27
 
 API changes, most recent first:
 
+2021-10-13 - xx - lavf 59.7.100 - avio.h
+  Deprecate AVIOContext.written. Originally added as a private entry in
+  commit 3f75e5116b900f1428aa13041fc7d6301bf1988a, its grouping with
+  the comment noting its private state was missed during merging of the field
+  from Libav (most likely due to an already existing field in between).
+
 2021-09-21 - xx - lavu 57.7.100 - pixfmt.h
   Add AV_PIX_FMT_X2BGR10.
 
diff --git a/libavformat/avio.h b/libavformat/avio.h
index a7b56ab667..5e60c2e35c 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -290,7 +290,13 @@ typedef struct AVIOContext {
  */
 int ignore_boundary_point;
 
+#if FF_API_AVIOCONTEXT_WRITTEN
+/**
+ * @deprecated field utilized privately by libavformat.
+ */
+attribute_deprecated
 int64_t written;
+#endif
 
 /**
  * Maximum reached position before a backward seek in the write buffer,
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index b18a56ef19..f21f1c89df 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -22,6 +22,7 @@
 #include "libavutil/bprint.h"
 #include "libavutil/crc.h"
 #include "libavutil/dict.h"
+#include "libavutil/internal.h"
 #include "libavutil/intreadwrite.h"
 #include "libavutil/log.h"
 #include "libavutil/opt.h"
@@ -124,7 +125,11 @@ void ffio_init_context(FFIOContext *ctx,
 ctx->current_type= AVIO_DATA_MARKER_UNKNOWN;
 ctx->last_time   = AV_NOPTS_VALUE;
 ctx->short_seek_get  = NULL;
+#if FF_API_AVIOCONTEXT_WRITTEN
+FF_DISABLE_DEPRECATION_WARNINGS
 s->written   = 0;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
 }
 
 AVIOContext *avio_alloc_context(
@@ -166,7 +171,11 @@ static void writeout(AVIOContext *s, const uint8_t *data, 
int len)
 } else {
 if (s->pos + len > ctx->written_output_size) {
 ctx->written_output_size = s->pos + len;
+#if FF_API_AVIOCONTEXT_WRITTEN
+FF_DISABLE_DEPRECATION_WARNINGS
 s->written = ctx->written_output_size;
+FF_ENABLE_DEPRECATION_WARNINGS
+#endif
 }
 }
 }
diff --git a/libavformat/version.h b/libavformat/version.h
index d5dd22059b..de780124c7 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  59
-#define LIBAVFORMAT_VERSION_MINOR   6
+#define LIBAVFORMAT_VERSION_MINOR   7
 #define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
@@ -61,6 +61,9 @@
 #ifndef FF_API_COMPUTE_PKT_FIELDS2
 #define FF_API_COMPUTE_PKT_FIELDS2  (LIBAVFORMAT_VERSION_MAJOR < 60)
 #endif
+#ifndef FF_API_AVIOCONTEXT_WRITTEN
+#define FF_API_AVIOCONTEXT_WRITTEN  (LIBAVFORMAT_VERSION_MAJOR < 60)
+#endif
 
 
 #ifndef FF_API_R_FRAME_RATE
-- 
2.31.1

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

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


[FFmpeg-devel] [PATCH v3 3/3] avformat/avio{, buf}: introduce public AVIOContext::bytes_{read, written}

2021-10-18 Thread Jan Ekström
Such fields can be seen as generally useful in cases where the
API user is not implementing custom AVIO callbacks, but still would
like to know if data is being read or written out, such as in case
data is being read from input but no AVPacket has been received yet.
---
 doc/APIchanges  |  3 +++
 libavformat/avio.h  | 14 +-
 libavformat/avio_internal.h |  5 +
 libavformat/aviobuf.c   | 10 --
 libavformat/version.h   |  2 +-
 5 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/doc/APIchanges b/doc/APIchanges
index 4731e14cb1..99e185ee4e 100644
--- a/doc/APIchanges
+++ b/doc/APIchanges
@@ -14,6 +14,9 @@ libavutil: 2021-04-27
 
 API changes, most recent first:
 
+2021-10-18 - xx - lavf 59.8.100 - avio.h
+  Introduce public bytes_{read,written} statistic fields to AVIOContext.
+
 2021-10-13 - xx - lavf 59.7.100 - avio.h
   Deprecate AVIOContext.written. Originally added as a private entry in
   commit 3f75e5116b900f1428aa13041fc7d6301bf1988a, its grouping with
diff --git a/libavformat/avio.h b/libavformat/avio.h
index 5e60c2e35c..cd63322a62 100644
--- a/libavformat/avio.h
+++ b/libavformat/avio.h
@@ -292,7 +292,9 @@ typedef struct AVIOContext {
 
 #if FF_API_AVIOCONTEXT_WRITTEN
 /**
- * @deprecated field utilized privately by libavformat.
+ * @deprecated field utilized privately by libavformat. For a public
+ * statistic of how many bytes were written out, see
+ * AVIOContext::bytes_written.
  */
 attribute_deprecated
 int64_t written;
@@ -303,6 +305,16 @@ typedef struct AVIOContext {
  * used keeping track of already written data for a later flush.
  */
 unsigned char *buf_ptr_max;
+
+/**
+ * Read-only statistic of bytes read for this AVIOContext.
+ */
+int64_t bytes_read;
+
+/**
+ * Read-only statistic of bytes written for this AVIOContext.
+ */
+int64_t bytes_written;
 } AVIOContext;
 
 /**
diff --git a/libavformat/avio_internal.h b/libavformat/avio_internal.h
index 467e80701f..187433f283 100644
--- a/libavformat/avio_internal.h
+++ b/libavformat/avio_internal.h
@@ -51,6 +51,11 @@ typedef struct FFIOContext {
  */
 int64_t bytes_read;
 
+/**
+ * Bytes written statistic
+ */
+int64_t bytes_written;
+
 /**
  * seek statistic
  */
diff --git a/libavformat/aviobuf.c b/libavformat/aviobuf.c
index f21f1c89df..5da4dea7b6 100644
--- a/libavformat/aviobuf.c
+++ b/libavformat/aviobuf.c
@@ -169,6 +169,9 @@ static void writeout(AVIOContext *s, const uint8_t *data, 
int len)
 if (ret < 0) {
 s->error = ret;
 } else {
+ctx->bytes_written += len;
+s->bytes_written = ctx->bytes_written;
+
 if (s->pos + len > ctx->written_output_size) {
 ctx->written_output_size = s->pos + len;
 #if FF_API_AVIOCONTEXT_WRITTEN
@@ -584,6 +587,7 @@ static void fill_buffer(AVIOContext *s)
 s->buf_ptr = dst;
 s->buf_end = dst + len;
 ffiocontext(s)->bytes_read += len;
+s->bytes_read = ffiocontext(s)->bytes_read;
 }
 }
 
@@ -657,6 +661,7 @@ int avio_read(AVIOContext *s, unsigned char *buf, int size)
 } else {
 s->pos += len;
 ffiocontext(s)->bytes_read += len;
+s->bytes_read = ffiocontext(s)->bytes_read;
 size -= len;
 buf += len;
 // reset the buffer
@@ -1236,8 +1241,9 @@ int avio_close(AVIOContext *s)
 
 av_freep(&s->buffer);
 if (s->write_flag)
-av_log(s, AV_LOG_VERBOSE, "Statistics: %d seeks, %d writeouts\n",
-   ctx->seek_count, ctx->writeout_count);
+av_log(s, AV_LOG_VERBOSE,
+   "Statistics: %"PRId64" bytes written, %d seeks, %d writeouts\n",
+   ctx->bytes_written, ctx->seek_count, ctx->writeout_count);
 else
 av_log(s, AV_LOG_VERBOSE, "Statistics: %"PRId64" bytes read, %d 
seeks\n",
ctx->bytes_read, ctx->seek_count);
diff --git a/libavformat/version.h b/libavformat/version.h
index de780124c7..81ed517609 100644
--- a/libavformat/version.h
+++ b/libavformat/version.h
@@ -32,7 +32,7 @@
 // Major bumping may affect Ticket5467, 5421, 5451(compatibility with Chromium)
 // Also please add any ticket numbers that you believe might be affected here
 #define LIBAVFORMAT_VERSION_MAJOR  59
-#define LIBAVFORMAT_VERSION_MINOR   7
+#define LIBAVFORMAT_VERSION_MINOR   8
 #define LIBAVFORMAT_VERSION_MICRO 100
 
 #define LIBAVFORMAT_VERSION_INT AV_VERSION_INT(LIBAVFORMAT_VERSION_MAJOR, \
-- 
2.31.1

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

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


Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/avio{, buf}: deprecate AVIOContext::written

2021-10-19 Thread Jan Ekström
On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström  wrote:
>
> Originally added as a private entry in commit
> 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> the comment noting its private state was missed during merging of
> the field from Libav (most likely due to an already existing field
> in between).
> ---

As noted in the previous two iterations' cover letter etc, I would
like to have some opinions on whether to remove or deprecate this
struct member.

Personally I see the struct member being one of those private ones
that Andreas removed earlier, and thus deprecating it feels weird
while all of the other private variables got removed from AVIOContext
already.

I can also do this deprecation dance, as this patch already shows that
change being done :) .

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


Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/avio{, buf}: deprecate AVIOContext::written

2021-10-21 Thread Jan Ekström
On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
 wrote:
>
> On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström  wrote:
> > >
> > > Originally added as a private entry in commit
> > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > the comment noting its private state was missed during merging of
> > > the field from Libav (most likely due to an already existing field
> > > in between).
> > > ---
>
> Its a public struct in a public header so deprecate
>
> thx
>

OK, just confirming but so you don't consider this to be of the same
type as these 
http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
?

The field was brought in with
http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=7bf7985c5e04a551e3a2841decc6b32ded7c0799;hp=49721aa1315fc44bf46d73d343ace4e7c3dc785e;hb=3f75e5116b900f1428aa13041fc7d6301bf1988a;hpb=fc85646ad495f3418042468da415af73a7a07334
(you can see the comment noting its private state on the top of the context)

which then was merged as
http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=525eb7129eb5b5a5a2a8dc015275526b9b25a0cb;hp=6f4ed8440d684e03e70fd296849b76d216d6fbbb;hb=34d7f337c1608c72be8c36355018dc894f0560ce;hpb=c5fd47fa8a300fc51489a47da94041609545803c
(it went just under another private struct member from the FFmpeg side)

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


Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/avio{, buf}: deprecate AVIOContext::written

2021-10-21 Thread Jan Ekström
On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
 wrote:
>
> On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> >  wrote:
> > >
> > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström  wrote:
> > > > >
> > > > > Originally added as a private entry in commit
> > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > > > the comment noting its private state was missed during merging of
> > > > > the field from Libav (most likely due to an already existing field
> > > > > in between).
> > > > > ---
> > >
> > > Its a public struct in a public header so deprecate
> > >
> > > thx
> > >
> >
> > OK, just confirming but so you don't consider this to be of the same
> > type as these 
> > http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > ?
>
> i assume this was during ABI bumping,
> when the major version numbers change then ABI/API breaking changes can be
> done.
>

IIRC ABI breakage after bumping was supposed to be a ~month but then
IIRC people posted such things much later. Was there a message or note
when this period ended?

> Try to write an application which links to a shared lib and then change the
> public structs of that lib by removing a field.
> the ABI/API major numbers have to be bumped when incompatible changes are
> done. That way they can co exist on a machiene, application build against
> the old continue working and so on ...
>

For the record, I know what ABI bumping is. I was just unclear whether
we were already past ABI instability period or not due to
4.5/5.0/whatever not still being made and due to there (as far as I
know) there being no clear message on when the ABI instability period
would end.

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


Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/avio{, buf}: deprecate AVIOContext::written

2021-10-21 Thread Jan Ekström
On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström  wrote:
>
> On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
>  wrote:
> >
> > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > >  wrote:
> > > >
> > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström  wrote:
> > > > > >
> > > > > > Originally added as a private entry in commit
> > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > > > > the comment noting its private state was missed during merging of
> > > > > > the field from Libav (most likely due to an already existing field
> > > > > > in between).
> > > > > > ---
> > > >
> > > > Its a public struct in a public header so deprecate
> > > >
> > > > thx
> > > >
> > >
> > > OK, just confirming but so you don't consider this to be of the same
> > > type as these 
> > > http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > ?
> >
> > i assume this was during ABI bumping,
> > when the major version numbers change then ABI/API breaking changes can be
> > done.
> >
>
> IIRC ABI breakage after bumping was supposed to be a ~month but then
> IIRC people posted such things much later. Was there a message or note
> when this period ended?
>

This commit 
(http://git.videolan.org/?p=ffmpeg.git;a=commit;h=45bfe8b838275235412777dd430206d9a24eb3ee)
was applied in late August (as the entries were private - kind of like
this one remaining entry), and I can find Anton noting in May that the
project is in ABI instability period. So do excuse me for not knowing
which mode we are in right now if it went on for at least four months
:D

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


Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/avio{, buf}: deprecate AVIOContext::written

2021-10-22 Thread Jan Ekström
On Fri, Oct 22, 2021 at 1:13 AM Michael Niedermayer
 wrote:
>
> On Fri, Oct 22, 2021 at 12:32:18AM +0300, Jan Ekström wrote:
> > On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström  wrote:
> > >
> > > On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> > >  wrote:
> > > >
> > > > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > > > >  wrote:
> > > > > >
> > > > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström  
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Originally added as a private entry in commit
> > > > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping with
> > > > > > > > the comment noting its private state was missed during merging 
> > > > > > > > of
> > > > > > > > the field from Libav (most likely due to an already existing 
> > > > > > > > field
> > > > > > > > in between).
> > > > > > > > ---
> > > > > >
> > > > > > Its a public struct in a public header so deprecate
> > > > > >
> > > > > > thx
> > > > > >
> > > > >
> > > > > OK, just confirming but so you don't consider this to be of the same
> > > > > type as these 
> > > > > http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > > > ?
> > > >
> > > > i assume this was during ABI bumping,
> > > > when the major version numbers change then ABI/API breaking changes can 
> > > > be
> > > > done.
> > > >
> > >
> > > IIRC ABI breakage after bumping was supposed to be a ~month but then
> > > IIRC people posted such things much later. Was there a message or note
> > > when this period ended?
>
> Maybe someone assumed that everyone agreed what "a ~month" is ?
>
>
> > >
> >
> > This commit 
> > (http://git.videolan.org/?p=ffmpeg.git;a=commit;h=45bfe8b838275235412777dd430206d9a24eb3ee)
> > was applied in late August (as the entries were private - kind of like
> > this one remaining entry), and I can find Anton noting in May that the
> > project is in ABI instability period. So do excuse me for not knowing
> > which mode we are in right now if it went on for at least four months
> > :D
>
> I think you should really implement a full statistics API instead of
> this as you clearly have alot of time.

I do not have a lot of time, I just saw this as a finalization of the
clean-up of private fields that Andreas started and I wanted to ask
about which people would prefer, since we have not yet had a release
branching with the API major version that this clean-up is included
in.

If you disagree with Andreas's actions from late August, then I would
have preferred you to note that. I would have been OK with something a
la "It was not clearly marked in our (FFmpeg's) merge so we consider
it public and not private.", instead I got what (at least to me) felt
like re-education on what API/ABI breaks are, plus what seemed to be
not reading all those references I provided to why I thought this was
a private field and why this in my opinion was just a continuation of
the previous commit done by Andreas. I am sorry for the knee-jerk
reflex-like response, it is not productive. I think I just felt like
getting ignored and getting a standard "he doesn't understand the
basics of this basic thing" response, even though I attempted to
provide all the information regarding *why* I was asking.

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


Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/avio{, buf}: deprecate AVIOContext::written

2021-10-22 Thread Jan Ekström
On Fri, Oct 22, 2021 at 6:16 PM Michael Niedermayer
 wrote:
>
> On Fri, Oct 22, 2021 at 12:20:19PM +0300, Jan Ekström wrote:
> > On Fri, Oct 22, 2021 at 1:13 AM Michael Niedermayer
> >  wrote:
> > >
> > > On Fri, Oct 22, 2021 at 12:32:18AM +0300, Jan Ekström wrote:
> > > > On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström  wrote:
> > > > >
> > > > > On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> > > > >  wrote:
> > > > > >
> > > > > > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > > > > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > > > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström 
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > Originally added as a private entry in commit
> > > > > > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its grouping 
> > > > > > > > > > with
> > > > > > > > > > the comment noting its private state was missed during 
> > > > > > > > > > merging of
> > > > > > > > > > the field from Libav (most likely due to an already 
> > > > > > > > > > existing field
> > > > > > > > > > in between).
> > > > > > > > > > ---
> > > > > > > >
> > > > > > > > Its a public struct in a public header so deprecate
> > > > > > > >
> > > > > > > > thx
> > > > > > > >
> > > > > > >
> > > > > > > OK, just confirming but so you don't consider this to be of the 
> > > > > > > same
> > > > > > > type as these 
> > > > > > > http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > > > > > ?
> > > > > >
> > > > > > i assume this was during ABI bumping,
> > > > > > when the major version numbers change then ABI/API breaking changes 
> > > > > > can be
> > > > > > done.
> > > > > >
> > > > >
> > > > > IIRC ABI breakage after bumping was supposed to be a ~month but then
> > > > > IIRC people posted such things much later. Was there a message or note
> > > > > when this period ended?
> > >
> > > Maybe someone assumed that everyone agreed what "a ~month" is ?
> > >
> > >
> > > > >
> > > >
> > > > This commit 
> > > > (http://git.videolan.org/?p=ffmpeg.git;a=commit;h=45bfe8b838275235412777dd430206d9a24eb3ee)
> > > > was applied in late August (as the entries were private - kind of like
> > > > this one remaining entry), and I can find Anton noting in May that the
> > > > project is in ABI instability period. So do excuse me for not knowing
> > > > which mode we are in right now if it went on for at least four months
> > > > :D
> > >
> > > I think you should really implement a full statistics API instead of
> > > this as you clearly have alot of time.
> >
> > I do not have a lot of time, I just saw this as a finalization of the
> > clean-up of private fields that Andreas started and I wanted to ask
> > about which people would prefer, since we have not yet had a release
> > branching with the API major version that this clean-up is included
> > in.
> >
> > If you disagree with Andreas's actions from late August, then I would
> > have preferred you to note that. I would have been OK with something a
> > la "It was not clearly marked in our (FFmpeg's) merge so we consider
> > it public and not private.", instead I got what (at least to me) felt
> > like re-education on what API/ABI breaks are, plus what seemed to be
> > not reading all those references I provided to why I thought this was
> > a private field and why this in my opinion was just a continuation of
> > the previous comm

Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/avio{, buf}: deprecate AVIOContext::written

2021-10-22 Thread Jan Ekström
On Fri, Oct 22, 2021 at 11:26 PM Michael Niedermayer
 wrote:
>
> On Fri, Oct 22, 2021 at 08:42:14PM +0300, Jan Ekström wrote:
> > On Fri, Oct 22, 2021 at 6:16 PM Michael Niedermayer
> >  wrote:
> > >
> > > On Fri, Oct 22, 2021 at 12:20:19PM +0300, Jan Ekström wrote:
> > > > On Fri, Oct 22, 2021 at 1:13 AM Michael Niedermayer
> > > >  wrote:
> > > > >
> > > > > On Fri, Oct 22, 2021 at 12:32:18AM +0300, Jan Ekström wrote:
> > > > > > On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström  
> > > > > > wrote:
> > > > > > >
> > > > > > > On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > > > > > > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström wrote:
> > > > > > > > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström 
> > > > > > > > > > >  wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Originally added as a private entry in commit
> > > > > > > > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its 
> > > > > > > > > > > > grouping with
> > > > > > > > > > > > the comment noting its private state was missed during 
> > > > > > > > > > > > merging of
> > > > > > > > > > > > the field from Libav (most likely due to an already 
> > > > > > > > > > > > existing field
> > > > > > > > > > > > in between).
> > > > > > > > > > > > ---
> > > > > > > > > >
> > > > > > > > > > Its a public struct in a public header so deprecate
> > > > > > > > > >
> > > > > > > > > > thx
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > OK, just confirming but so you don't consider this to be of 
> > > > > > > > > the same
> > > > > > > > > type as these 
> > > > > > > > > http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > > > > > > > ?
> > > > > > > >
> > > > > > > > i assume this was during ABI bumping,
> > > > > > > > when the major version numbers change then ABI/API breaking 
> > > > > > > > changes can be
> > > > > > > > done.
> > > > > > > >
> > > > > > >
> > > > > > > IIRC ABI breakage after bumping was supposed to be a ~month but 
> > > > > > > then
> > > > > > > IIRC people posted such things much later. Was there a message or 
> > > > > > > note
> > > > > > > when this period ended?
> > > > >
> > > > > Maybe someone assumed that everyone agreed what "a ~month" is ?
> > > > >
> > > > >
> > > > > > >
> > > > > >
> > > > > > This commit 
> > > > > > (http://git.videolan.org/?p=ffmpeg.git;a=commit;h=45bfe8b838275235412777dd430206d9a24eb3ee)
> > > > > > was applied in late August (as the entries were private - kind of 
> > > > > > like
> > > > > > this one remaining entry), and I can find Anton noting in May that 
> > > > > > the
> > > > > > project is in ABI instability period. So do excuse me for not 
> > > > > > knowing
> > > > > > which mode we are in right now if it went on for at least four 
> > > > > > months
> > > > > > :D
> > > > >
> > > > > I think you should really implement a full statistics API 

Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/avio{, buf}: deprecate AVIOContext::written

2021-10-22 Thread Jan Ekström
On Sat, Oct 23, 2021 at 12:11 AM Jan Ekström  wrote:
>
> On Fri, Oct 22, 2021 at 11:26 PM Michael Niedermayer
>  wrote:
> >
> > On Fri, Oct 22, 2021 at 08:42:14PM +0300, Jan Ekström wrote:
> > > On Fri, Oct 22, 2021 at 6:16 PM Michael Niedermayer
> > >  wrote:
> > > >
> > > > On Fri, Oct 22, 2021 at 12:20:19PM +0300, Jan Ekström wrote:
> > > > > On Fri, Oct 22, 2021 at 1:13 AM Michael Niedermayer
> > > > >  wrote:
> > > > > >
> > > > > > On Fri, Oct 22, 2021 at 12:32:18AM +0300, Jan Ekström wrote:
> > > > > > > On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström  
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > > > > > > > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > > > > > > > > >  wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström 
> > > > > > > > > > > wrote:
> > > > > > > > > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström 
> > > > > > > > > > > >  wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Originally added as a private entry in commit
> > > > > > > > > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its 
> > > > > > > > > > > > > grouping with
> > > > > > > > > > > > > the comment noting its private state was missed 
> > > > > > > > > > > > > during merging of
> > > > > > > > > > > > > the field from Libav (most likely due to an already 
> > > > > > > > > > > > > existing field
> > > > > > > > > > > > > in between).
> > > > > > > > > > > > > ---
> > > > > > > > > > >
> > > > > > > > > > > Its a public struct in a public header so deprecate
> > > > > > > > > > >
> > > > > > > > > > > thx
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > OK, just confirming but so you don't consider this to be of 
> > > > > > > > > > the same
> > > > > > > > > > type as these 
> > > > > > > > > > http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > > > > > > > > ?
> > > > > > > > >
> > > > > > > > > i assume this was during ABI bumping,
> > > > > > > > > when the major version numbers change then ABI/API breaking 
> > > > > > > > > changes can be
> > > > > > > > > done.
> > > > > > > > >
> > > > > > > >
> > > > > > > > IIRC ABI breakage after bumping was supposed to be a ~month but 
> > > > > > > > then
> > > > > > > > IIRC people posted such things much later. Was there a message 
> > > > > > > > or note
> > > > > > > > when this period ended?
> > > > > >
> > > > > > Maybe someone assumed that everyone agreed what "a ~month" is ?
> > > > > >
> > > > > >
> > > > > > > >
> > > > > > >
> > > > > > > This commit 
> > > > > > > (http://git.videolan.org/?p=ffmpeg.git;a=commit;h=45bfe8b838275235412777dd430206d9a24eb3ee)
> > > > > > > was applied in late August (as the entries were private - kind of 
> > > > > > > like
> > > > > > > this one remaining entry), and I can find Ant

Re: [FFmpeg-devel] [PATCH v3 2/3] avformat/avio{, buf}: deprecate AVIOContext::written

2021-10-22 Thread Jan Ekström
On Sat, Oct 23, 2021 at 12:13 AM Jan Ekström  wrote:
>
> On Sat, Oct 23, 2021 at 12:11 AM Jan Ekström  wrote:
> >
> > On Fri, Oct 22, 2021 at 11:26 PM Michael Niedermayer
> >  wrote:
> > >
> > > On Fri, Oct 22, 2021 at 08:42:14PM +0300, Jan Ekström wrote:
> > > > On Fri, Oct 22, 2021 at 6:16 PM Michael Niedermayer
> > > >  wrote:
> > > > >
> > > > > On Fri, Oct 22, 2021 at 12:20:19PM +0300, Jan Ekström wrote:
> > > > > > On Fri, Oct 22, 2021 at 1:13 AM Michael Niedermayer
> > > > > >  wrote:
> > > > > > >
> > > > > > > On Fri, Oct 22, 2021 at 12:32:18AM +0300, Jan Ekström wrote:
> > > > > > > > On Fri, Oct 22, 2021 at 12:21 AM Jan Ekström  
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Thu, Oct 21, 2021 at 11:49 PM Michael Niedermayer
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > On Thu, Oct 21, 2021 at 08:48:35PM +0300, Jan Ekström wrote:
> > > > > > > > > > > On Thu, Oct 21, 2021 at 8:26 PM Michael Niedermayer
> > > > > > > > > > >  wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, Oct 20, 2021 at 12:02:13AM +0300, Jan Ekström 
> > > > > > > > > > > > wrote:
> > > > > > > > > > > > > On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström 
> > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Originally added as a private entry in commit
> > > > > > > > > > > > > > 3f75e5116b900f1428aa13041fc7d6301bf1988a, but its 
> > > > > > > > > > > > > > grouping with
> > > > > > > > > > > > > > the comment noting its private state was missed 
> > > > > > > > > > > > > > during merging of
> > > > > > > > > > > > > > the field from Libav (most likely due to an already 
> > > > > > > > > > > > > > existing field
> > > > > > > > > > > > > > in between).
> > > > > > > > > > > > > > ---
> > > > > > > > > > > >
> > > > > > > > > > > > Its a public struct in a public header so deprecate
> > > > > > > > > > > >
> > > > > > > > > > > > thx
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > OK, just confirming but so you don't consider this to be 
> > > > > > > > > > > of the same
> > > > > > > > > > > type as these 
> > > > > > > > > > > http://git.videolan.org/?p=ffmpeg.git;a=blobdiff;f=libavformat/avio.h;h=a7b56ab6674f0a25be3b480df314e2d1292f397b;hp=0b35409787c925d681a121aa8d8715c3921fddf2;hb=45bfe8b838275235412777dd430206d9a24eb3ee;hpb=530ac6aa305aeda631c77f8a17e96c14c7ab1a1c
> > > > > > > > > > > ?
> > > > > > > > > >
> > > > > > > > > > i assume this was during ABI bumping,
> > > > > > > > > > when the major version numbers change then ABI/API breaking 
> > > > > > > > > > changes can be
> > > > > > > > > > done.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > IIRC ABI breakage after bumping was supposed to be a ~month 
> > > > > > > > > but then
> > > > > > > > > IIRC people posted such things much later. Was there a 
> > > > > > > > > message or note
> > > > > > > > > when this period ended?
> > > > > > >
> > > > > > > Maybe someone assumed that everyone agreed what "a ~month" is ?
> > > > > > >
> > > > > > >
> > > > > > > > >
> > > > > > > >
&g

Re: [FFmpeg-devel] [PATCH 1/3] avutil: Add Dolby Vision RPU side data type

2021-10-22 Thread Jan Ekström
On Thu, Oct 21, 2021 at 4:49 PM Derek Buitenhuis
 wrote:
>
> Signed-off-by: Derek Buitenhuis 
> ---
> All known encoders, etc. expect RPUs in this format, to my knowledge.
> (There is no spec for parsing the RPUs, even privately, and not a single
> piece of software that would benefit from that

DoVi Profile 5 and 7 rendering requires the data within these RPU
entities, AFAIK (as it contains various values to correctly configure
the custom color space). So software such as libplacebo where haasn
has made efforts towards implementing the non-standard color space
utilized would benefit from having the contents.

So while there clearly are two separate use cases, I would note that
saying "nothing would see any use from the bytes within these
structures" is maybe a bit disingenuous.

The two use cases being:
1. Just re-encoding the content. The contents of these RPU buffers
being unrelated and only passthrough is necessary.
2. Trying to gather the information of these metadata units regarding
interpretation of the received image.

Both are clearly valid use cases, and we should IMHO support both, at
least on the AVFrame level. Not that I mean that you should implement
the parsing of these metadata units, since it is clear you want to
keep away from that part of the work :) .

> - and it is a legal minefield
> which I am not going to touch.)

While with this I agree with completely. Or at least given the D
company's track record :P . That said, just parsing the metadata units
is probably much less of a swamp than actually trying to utilize the
metadata to properly render Profile 5 or 7 video.

For the record, there are at least two known to me OSS implementations
of RPU parsing/writing, one being made in Java and the other being
made in Rust.

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


Re: [FFmpeg-devel] [PATCH 2/3] avcodec/hevcdec: Export Dolby Vision RPUs as side data

2021-10-23 Thread Jan Ekström
On Sat, Oct 23, 2021 at 9:16 PM James Almer  wrote:
>
> On 10/23/2021 2:52 PM, Derek Buitenhuis wrote:
> > On 10/23/2021 3:18 PM, James Almer wrote:
> >> 0x7c01 is just the NAL header that signals type 62, nuh_layer_id 0 and
> >> nuh_temporal_id 0.
> >>
> >> forbidden_zero_bit=  0
> >> nal_unit_type = 10
> >> nuh_layer_id  = 00
> >> nuh_temporal_id_plus1 =001
> >>
> >> So the last two checks can be changed to !pkt->nals[pkt->nb_nals -
> >> 1].nuh_layer_id && !pkt->nals[pkt->nb_nals - 1].temporal_id
> >
> > Do'h, will fix.
>
> That being said, would Dolby RPU NALUs like this using other values for
> temporal and layer id be valid? Or can we just assume that's never
> happening?
>
> >
> >> This is horrible, but i guess there's no alternative without
> >> autoinserting a bsf.
> >
> > Yeah, I couldn't think of one - and the BSF option seemed even worse.
> >
> >> What i said above. It's nothing Dolby specific. Any NAL type 62 could be
> >> like that. If there's no spec we could use to actually parse the
> >> bitstream after the NAL header, then we will end up propagating pretty
> >> much anything using NAL type 62 as Dolby RPU.
> >
> > Indeed, there is no spec. As far as I know, nothing else uses UNSPEC62,
> > though, and this is consistent with what e.g. Dolby's open source muxer
> > and mkvtoolnix do.
> >
> > What do you suggest? Rename the side data to _UNSPEC62 maybe and leave it
> > to the caller to handle it?
>
> No, i prefer having the side data be about Dolby Vision RPU. Who knows,
> maybe in the future Dolby realizes they had the Unregistered and even
> Registered User Data SEI readily available for this kind of proprietary
> information and start using that instead...
>

They already utilize something like this for their ST.2094-10
brightness metadata (which is actually specified through ETSI etc
[1]), which hilariously is also called "DoVi" in their marketing.

Since effectively D has taken over NAL unit 62 for this stuff, just
call it RPU_BUFFER or so, and take in everything NAL unit 62?

Jan

[1] 
https://www.etsi.org/deliver/etsi_ts/103500_103599/103572/01.03.01_60/ts_103572v010301p.pdf
___
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 v3 0/3] introduce public AVIOContext::bytes_{read, written}

2021-10-24 Thread Jan Ekström
On Mon, Oct 18, 2021 at 3:47 PM Jan Ekström  wrote:
>
> Changes compared to v2:
> * Written was actually written_size, so it did not take into account any
>   writes after a seek-back. Thus an initial attempt at implementing
>   bytes_written was made.
>
> After a brief discussion with Michael on IRC, this seems to be the idea of a
> path forward that was agreed upon.
>
> 1. AVIOContext::written was supposed to be a private field [1], so move the
>value utilized internally to FFIOContext, and set the AVIOContext value
>from there.
> 2. Deprecate AVIOContext::written.
> 3. Introduce public AVIOContext::bytes_{read,written} statistics fields.
>
> I was not sure whether deprecation or straight-out removal was the right thing
> to do - since while the field was meant to be internal it was not marked as
> such in FFmpeg's merged state of the struct (which is why it did not get
> cleaned up into FFIOContext earlier) - but in order to not get stuck on that,
> I am now posting this with the full deprecation changes. This way (hopefully)
> this change will get more eyeballs and if someone thinks this could just be
> removed the patches can be changed to do that instead.
>
> [1] 
> http://git.videolan.org/?p=ffmpeg.git;a=commit;h=3f75e5116b900f1428aa13041fc7d6301bf1988a
>

Thanks for the comments, applied the set as
d39b58dc32b5fc7b480eeb9ef00a610732f02c2c
a5622ed16f8e22a80cecd8936799e61f61a74cd5
682bafdb12507ec8b049ecbbe2e48bf814927002

with one minor change, "point of truth" -> "source of truth" in the
first commit.

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


Re: [FFmpeg-devel] [PATCH 7/8] avformat/mpegts: Fix for the DOVI video stream descriptor

2021-10-27 Thread Jan Ekström
On Thu, Oct 14, 2021 at 5:31 PM  wrote:
>
> On Thu, Oct 14, 2021 at 10:12:05AM -0400, quietvoid wrote:
> > On Thu, Oct 14, 2021 at 9:52 AM  wrote:
> >
> > > On Thu, Oct 14, 2021 at 09:45:45AM -0400, quietvoid wrote:
> > > > On Thu, Oct 14, 2021 at 9:36 AM  wrote:
> > > >
> > > > > On Thu, Oct 14, 2021 at 09:18:16AM -0400, f tcChlisop0 wrote:
> > > > > > On Thu, Oct 14, 2021 at 9:10 AM  wrote:
> > > > > >
> > > > > > > From: Limin Wang 
> > > > > > >
> > > > > > > By < > > > > > > Format
> > > > > v1.2>>
> > > > > > >
> > > > > > > Signed-off-by: Limin Wang 
> > > > > > > ---
> > > > > > >  libavformat/mpegts.c | 11 +--
> > > > > > >  1 file changed, 9 insertions(+), 2 deletions(-)
> > > > > > >
> > > > > > > diff --git a/libavformat/mpegts.c b/libavformat/mpegts.c
> > > > > > > index 44d9298..774964d 100644
> > > > > > > --- a/libavformat/mpegts.c
> > > > > > > +++ b/libavformat/mpegts.c
> > > > > > > @@ -2178,6 +2178,8 @@ int 
> > > > > > > ff_parse_mpeg2_descriptor(AVFormatContext
> > > > > *fc,
> > > > > > > AVStream *st, int stream_type
> > > > > > >  AVDOVIDecoderConfigurationRecord *dovi;
> > > > > > >  size_t dovi_size;
> > > > > > >  int ret;
> > > > > > > +int dependency_pid;
> > > > > > > +
> > > > > > >  if (desc_end - *pp < 4) // (8 + 8 + 7 + 6 + 1 + 1 +
> > > 1) / 8
> > > > > > >  return AVERROR_INVALIDDATA;
> > > > > > >
> > > > > > > @@ -2193,7 +2195,11 @@ int
> > > ff_parse_mpeg2_descriptor(AVFormatContext
> > > > > *fc,
> > > > > > > AVStream *st, int stream_type
> > > > > > >  dovi->rpu_present_flag  = (buf >> 2) & 0x01;// 1
> > > bit
> > > > > > >  dovi->el_present_flag   = (buf >> 1) & 0x01;// 1
> > > bit
> > > > > > >  dovi->bl_present_flag   =  buf   & 0x01;// 1
> > > bit
> > > > > > > -if (desc_end - *pp >= 20) {  // 4 + 4 * 4
> > > > > > > +if (!dovi->bl_present_flag && desc_end - *pp >= 2) {
> > > > > > > +buf = get16(pp, desc_end);
> > > > > > > +dependency_pid = buf >> 3; // 13 bits
> > > > > > > +}
> > > > > > > +if (desc_end - *pp >= 1) {  // 8 bits
> > > > > > >  buf = get8(pp, desc_end);
> > > > > > >  dovi->dv_bl_signal_compatibility_id = (buf >> 4) 
> > > > > > > &
> > > > > 0x0f;
> > > > > > > // 4 bits
> > > > > > >  } else {
> > > > > > > @@ -2210,12 +2216,13 @@ int
> > > ff_parse_mpeg2_descriptor(AVFormatContext
> > > > > *fc,
> > > > > > > AVStream *st, int stream_type
> > > > > > >  }
> > > > > > >
> > > > > > >  av_log(fc, AV_LOG_TRACE, "DOVI, version: %d.%d,
> > > profile:
> > > > > %d,
> > > > > > > level: %d, "
> > > > > > > -   "rpu flag: %d, el flag: %d, bl flag: %d,
> > > > > compatibility
> > > > > > > id: %d\n",
> > > > > > > +   "rpu flag: %d, el flag: %d, bl flag: %d,
> > > > > > > dependency_pid: %d, compatibility id: %d\n",
> > > > > > > dovi->dv_version_major, 
> > > > > > > dovi->dv_version_minor,
> > > > > > > dovi->dv_profile, dovi->dv_level,
> > > > > > > dovi->rpu_present_flag,
> > > > > > > dovi->el_present_flag,
> > > > > > > dovi->bl_present_flag,
> > > > > > > +   dependency_pid,
> > > > > > > dovi->dv_bl_signal_compatibility_id);
> > > > > > >  }
> > > > > > >  break;
> > > > > > > --
> > > > > > > 1.8.3.1
> > > > > > >
> > > > > > > ___
> > > > > > > ffmpeg-devel mailing list
> > > > > > > ffmpeg-devel@ffmpeg.org
> > > > > > > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> > > > > > >
> > > > > > > To unsubscribe, visit link above, or email
> > > > > > > ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
> > > > > > >
> > > > > >
> > > > > > Hi, this is something I had fixed in this patchset:
> > > > > > https://ffmpeg.org/pipermail/ffmpeg-devel/2021-September/286234.html
> > > > > > However the dependency_pid is ignored, as it has no use presently.
> > > > > >
> > > > > > Which patch should take precedence?
> > > > >
> > > > > Sorry, I have noticed your patch before. By the quick review of your
> > > patch,
> > > > > it's a lot function change and difficult to merge I think. I prefer to
> > > > > fix the issue with existing code first instead of mixed function
> > > change.
> > > > >
> > > >
> > > > Okay, that makes sense.
> > > > I will wait and rebase before resending for review then.
> > > >
> > > > However I'm worried my patch will still result in ignoring
> > > dependency_pid,
> > > > because it is not part of AVDOVIDecoderConfigurationRecord, unless it is
> > > > added.
> > >
> > > I failed to find your patchset in my email archive, so I reply it here for
> > > my comments:
> > > -if (ret < 0) {
> > > -   

[FFmpeg-devel] [PATCH 1/2] avfilter/vf_zscale: deduplicate output color information setting

2021-10-27 Thread Jan Ekström
This way a piece of logic is not missed in one location or the other,
such as the case with chroma location outside the if.
---
 libavfilter/vf_zscale.c | 45 ++---
 1 file changed, 20 insertions(+), 25 deletions(-)

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 3f7dba489a..439c0c8548 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -554,6 +554,24 @@ fail:
 return ret;
 }
 
+static void update_output_color_information(ZScaleContext *s, AVFrame *frame)
+{
+if (s->colorspace != -1)
+frame->colorspace = (int)s->dst_format.matrix_coefficients;
+
+if (s->primaries != -1)
+frame->color_primaries = (int)s->dst_format.color_primaries;
+
+if (s->range != -1)
+frame->color_range = 
convert_range_from_zimg(s->dst_format.pixel_range);
+
+if (s->trc != -1)
+frame->color_trc = (int)s->dst_format.transfer_characteristics;
+
+if (s->chromal != -1)
+frame->chroma_location = (int)s->dst_format.chroma_location - 1;
+}
+
 static int filter_frame(AVFilterLink *link, AVFrame *in)
 {
 ZScaleContext *s = link->dst->priv;
@@ -621,20 +639,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
 format_init(&s->dst_format, out, odesc, s->colorspace,
 s->primaries, s->trc, s->range, s->chromal);
 
-if (s->colorspace != -1)
-out->colorspace = (int)s->dst_format.matrix_coefficients;
-
-if (s->primaries != -1)
-out->color_primaries = (int)s->dst_format.color_primaries;
-
-if (s->range != -1)
-out->color_range = 
convert_range_from_zimg(s->dst_format.pixel_range);
-
-if (s->trc != -1)
-out->color_trc = (int)s->dst_format.transfer_characteristics;
-
-if (s->chromal != -1)
-out->chroma_location = (int)s->dst_format.chroma_location - 1;
+update_output_color_information(s, out);
 
 ret = graph_build(&s->graph, &s->params, &s->src_format, 
&s->dst_format,
   &s->tmp, &s->tmp_size);
@@ -680,17 +685,7 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
 }
 }
 
-if (s->colorspace != -1)
-out->colorspace = (int)s->dst_format.matrix_coefficients;
-
-if (s->primaries != -1)
-out->color_primaries = (int)s->dst_format.color_primaries;
-
-if (s->range != -1)
-out->color_range = convert_range_from_zimg(s->dst_format.pixel_range);
-
-if (s->trc != -1)
-out->color_trc = (int)s->dst_format.transfer_characteristics;
+update_output_color_information(s, out);
 
 av_reduce(&out->sample_aspect_ratio.num, &out->sample_aspect_ratio.den,
   (int64_t)in->sample_aspect_ratio.num * outlink->h * link->w,
-- 
2.31.1

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

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


[FFmpeg-devel] [PATCH 2/2] avfilter/vf_zscale: fix mapping of zimg_chroma_location_e to AVChromaLocation

2021-10-27 Thread Jan Ekström
The AVChromaLocation values are one higher than zimg's, not one
lower as the undefined value is set to zero (as opposed to zimg's
-1).
---
 libavfilter/vf_zscale.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 439c0c8548..1288c5efc1 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -569,7 +569,7 @@ static void update_output_color_information(ZScaleContext 
*s, AVFrame *frame)
 frame->color_trc = (int)s->dst_format.transfer_characteristics;
 
 if (s->chromal != -1)
-frame->chroma_location = (int)s->dst_format.chroma_location - 1;
+frame->chroma_location = (int)s->dst_format.chroma_location + 1;
 }
 
 static int filter_frame(AVFilterLink *link, AVFrame *in)
-- 
2.31.1

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

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


Re: [FFmpeg-devel] [PATCH 2/2] avfilter/vf_zscale: fix mapping of zimg_chroma_location_e to AVChromaLocation

2021-10-28 Thread Jan Ekström
On Thu, Oct 28, 2021 at 1:25 AM Paul B Mahol  wrote:
>
> probably fine

This can be verified by specifying - say - chromal topleft, and seeing
that the ffmpeg.c Output: bit right now would end up being "left"
(most likely). While if you apply this, you should get "topleft".

Technically if the values were unsanitized, having a mapping function
like we have the other way would be better, but since the code already
utilized +/- 1... I went with that way of just fixing the logic to go
the right way.

For verification of the internal values, something like the following
can be utilized:

diff --git a/libavfilter/vf_zscale.c b/libavfilter/vf_zscale.c
index 1288c5efc1..50df90e9ec 100644
--- a/libavfilter/vf_zscale.c
+++ b/libavfilter/vf_zscale.c
@@ -126,6 +126,8 @@ typedef struct ZScaleContext {
 enum AVColorPrimaries in_primaries, out_primaries;
 enum AVColorRange in_range, out_range;
 enum AVChromaLocation in_chromal, out_chromal;
+
+int lel;
 } ZScaleContext;

 static av_cold int init(AVFilterContext *ctx)
@@ -687,6 +689,12 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)

 update_output_color_information(s, out);

+av_log_once(s, AV_LOG_VERBOSE, AV_LOG_TRACE, &(s->lel),
+"output frame chroma location: %s "
+"(s->chromal: %d, dst_format.chroma_location: %d)\n",
+av_chroma_location_name(out->chroma_location),
+s->chromal, s->dst_format.chroma_location);
+
 av_reduce(&out->sample_aspect_ratio.num, &out->sample_aspect_ratio.den,
   (int64_t)in->sample_aspect_ratio.num * outlink->h * link->w,
   (int64_t)in->sample_aspect_ratio.den * outlink->w * link->h,
___
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] avfilter/vf_zscale: deduplicate output color information setting

2021-10-28 Thread Jan Ekström
On Thu, Oct 28, 2021 at 1:24 AM Paul B Mahol  wrote:
>
> lgtm

Thanks for the reviews, applied these patches as
cd1d09e81b53d47380b494acd3432fd4abb3c17b and
27c0dd55609daf440a7744e96ac20c119bbeb80f .

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


Re: [FFmpeg-devel] [PATCH 16/17] fftools/ffmpeg_opt: Fix copyinkf option name in warning message

2021-11-10 Thread Jan Ekström
On Tue, Nov 9, 2021 at 8:05 PM Andreas Rheinhardt
 wrote:
>
> Signed-off-by: Andreas Rheinhardt 

LGTM.

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


Re: [FFmpeg-devel] [PATCH 17/17] fftools/ffmpeg_opt: Apply copyinkf for all stream types

2021-11-10 Thread Jan Ekström
On Tue, Nov 9, 2021 at 8:05 PM Andreas Rheinhardt
 wrote:
>
> The earlier code has ignored it for all stream types except
> video and subtitles, probably because audio was presumed
> to only consist of keyframes. Yet this assumption is not true
> for e.g. TrueHD.
>
> Signed-off-by: Andreas Rheinhardt 
> ---
>  fftools/ffmpeg_opt.c | 7 +++
>  1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/fftools/ffmpeg_opt.c b/fftools/ffmpeg_opt.c
> index ab4c63a362..60ee6b16b5 100644
> --- a/fftools/ffmpeg_opt.c
> +++ b/fftools/ffmpeg_opt.c
> @@ -1655,6 +1655,9 @@ static OutputStream *new_output_stream(OptionsContext 
> *o, AVFormatContext *oc, e
>  ost->muxing_queue = av_fifo_alloc(8 * sizeof(AVPacket));
>  if (!ost->muxing_queue)
>  exit_program(1);
> +if (ost->stream_copy)
> +MATCH_PER_STREAM_OPT(copy_initial_nonkeyframes, i,
> + ost->copy_initial_nonkeyframes, oc, st);

Could use an extra empty line after the previous if that leads to
exit_program since the two are not related/grouped.

I think we can follow the way it was done for subtitle streams
previously, and just skip the ost->stream_copy check? There is no
extra initialization done for this logic, and having the correct value
always set in "ost" should not be a bad thing (unless I've missed
something with my quick check-around)

Otherwise LGTM for me.

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


Re: [FFmpeg-devel] [PATCH] avcodec/libkvazaar: Bump minimum version to 2.0.0

2023-10-11 Thread Jan Ekström
On Mon, Oct 2, 2023 at 8:04 PM John Mather via ffmpeg-devel
 wrote:
>
> 0cd8769207f utilized the rc_algorithm member of the kvz_config struct, which
> was introduced in Kvazaar 2.0.0. This patch bumps the minimum version of
> Kvazaar to 2.0.0 so that FFmpeg compiles successfully.
>
> Signed-off-by: John Mather 

LGTM.

Verified that the change indeed occurred between 1.3.0 and 2.0.0 and
that with this change 1.3.0 does not pass configure, and 2.0.0 both
passes configure as well as builds.

Applied as 7251dfdcee2f2ec83d9de10334257531762fe42b to master, and
then remembered that the description after prefix should have been
lowercase .-. .

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


Re: [FFmpeg-devel] [PATCH v3] avcodec/libkvazaar: Respect codec context color settings.

2023-10-11 Thread Jan Ekström
On Mon, Oct 2, 2023 at 8:21 PM John Mather via ffmpeg-devel
 wrote:
>
> This patch makes the libkvazaar encoder respect color settings that are
> present on the codec context, including color range, primaries, transfer
> function and colorspace.
> ---

LGTM.

Verified that this indeed builds with libkvazaar v2.0.0 and that the
color information gets properly passed.

Adjusted the equality signs to be on the same level and applied as
a2175ca8615d09418564fcff63c85e18dd993ad1 to master. And then same
thing with regards to the description starting with lowercase.

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


Re: [FFmpeg-devel] [PATCH v2] lavc/libaribcaption.c: add MSZ characters related options

2023-10-11 Thread Jan Ekström
On Tue, Oct 10, 2023 at 1:29 PM TADANO Tokumei  wrote:
>
> ping!
>
> On 2023/09/29 19:31, TADANO Tokumei wrote:
> >
> > On 2023/09/23 11:15, TADANO Tokumei wrote:
> >> This is updated patch to "[PATCH] lavc/libaribcaption.c: add 
> >> -replace_fullwidth_japanese option" (Message-Id: 
> >> <20230908130050.85688-1-aiming...@pc.nifty.jp>).
> >>
> >> If specified fonts contain half-width glyphs, it make better rendering with
> >> `-replace_msz_ascii false -replace_msz_japanese false` option for bitmap 
> >> sub_type.
> >>
> >> This patch also fix a bug in libaribcaption.c. I prefer to apply this ASAP.
> >>
> >> On 2023/09/23 11:00, TADANO Tokumei wrote:
> >>> This patch add MSZ (Middle Size; half width) characters related
> >>> options.
> >>>
> >>> * add `-replace_msz_japanese` option introduced in version 1.0.1
> >>>of libaribcaption.
> >>> * add `-replace_msz_glyph` option introduced in version 1.1.0
> >>>of libaribcaption.
> >>> * rename `-replace_fullwidth_ascii` option to `-replace_msz_ascii`
> >>>to clarify option meaning.
> >>> * FIX: change all the `bool` option variables in `ARIBCaptionContext`
> >>>to `int`.
> >>>On some environments, a `bool` variable is small space than `int`.
> >>>If a `bool` option was specified by command line, following
> >>>variables would be filled and destroyed.
> >>> ---
> >>>   doc/decoders.texi   | 28 --
> >>>   libavcodec/libaribcaption.c | 40 -
> >>>   2 files changed, 53 insertions(+), 15 deletions(-)
> >>>
> >>> diff --git a/doc/decoders.texi b/doc/decoders.texi
> >>> index 09b8314dd2..27b981b267 100644
> >>> --- a/doc/decoders.texi
> >>> +++ b/doc/decoders.texi
> >>> @@ -427,12 +427,6 @@ If your player cannot handle AVSubtitles with 
> >>> multiple ASS rectangles properly,
> >>>   set this option to @var{true} or define @env{ASS_SINGLE_RECT=1} to 
> >>> change
> >>>   default behavior at compilation.
> >>> -@item -replace_fullwidth_ascii @var{boolean}
> >>> -Specify whether to replace MSZ (Middle Size, half width) fullwidth
> >>> -alphanumerics with halfwidth alphanumerics.
> >>> -
> >>> -The default is @var{true}.
> >>> -
> >>>   @item -force_outline_text @var{boolean}
> >>>   Specify whether always render outline text for all characters 
> >>> regardless of
> >>>   the indication by charactor style.
> >>> @@ -459,6 +453,28 @@ Specify whether to render replaced DRCS characters 
> >>> as Unicode characters.
> >>>   The default is @var{true}.
> >>> +@item -replace_msz_ascii @var{boolean}
> >>> +Specify whether to replace MSZ (Middle Size, half width) fullwidth
> >>> +alphanumerics with halfwidth alphanumerics.
> >>> +
> >>> +The default is @var{true}.
> >>> +
> >>> +@item -replace_msz_japanese @var{boolean}
> >>> +Specify whether to replace some MSZ (Middle Size, half width) fullwidth
> >>> +japanese special characters with halfwidth ones.
> >>> +
> >>> +The default is @var{true}.
> >>> +
> >>> +@item -replace_msz_glyph @var{boolean}
> >>> +Specify whether to replace MSZ (Middle Size, half width) characters
> >>> +with halfwidth glyphs if the fonts supports it.
> >>> +This option works under FreeType or DirectWrite renderer
> >>> +with Adobe-Japan1 compliant fonts.
> >>> +e.g., IBM Plex Sans JP, Morisawa BIZ UDGothic, Morisawa BIZ UDMincho,
> >>> +Yu Gothic, Yu Mincho, and Meiryo.
> >>> +
> >>> +The default is @var{true}.
> >>> +
> >>>   @item -canvas_size @var{image_size}
> >>>   Specify the resolution of the canvas to render subtitles to; usually, 
> >>> this
> >>>   should be frame size of input video.
> >>> diff --git a/libavcodec/libaribcaption.c b/libavcodec/libaribcaption.c
> >>> index 8a8c8f8cfd..ddff47633a 100644
> >>> --- a/libavcodec/libaribcaption.c
> >>> +++ b/libavcodec/libaribcaption.c
> >>> @@ -68,14 +68,20 @@ typedef struct ARIBCaptionContext {
> >>>   int subtitle_type;
> >>>   int encoding_scheme;
> >>> -bool ass_single_rect;
> >>> +int ass_single_rect;
> >>>   char *font;
> >>> -bool replace_fullwidth_ascii;
> >>> -bool force_stroke_text;
> >>> -bool ignore_background;
> >>> -bool ignore_ruby;
> >>> +int force_stroke_text;
> >>> +int ignore_background;
> >>> +int ignore_ruby;
> >>>   float stroke_width;
> >>> -bool replace_drcs;
> >>> +int replace_drcs;
> >>> +int replace_msz_ascii;
> >>> +#if defined(ARIBCC_VERSION)
> >>> +int replace_msz_japanese;
> >>> +#  if AV_VERSION_INT(ARIBCC_VERSION_MAJOR, ARIBCC_VERSION_MINOR, 
> >>> ARIBCC_VERSION_PATCH) >= AV_VERSION_INT(1, 1, 0)
> >>> +int replace_msz_glyph;
> >>> +#  endif
> >>> +#endif
> >>>   int64_t pts;
> >>>   AVRational time_base;
> >>> @@ -1004,7 +1010,11 @@ static int aribcaption_init(AVCodecContext *avctx)
> >>>   return AVERROR_EXTERNAL;
> >>>   }
> >>>   aribcc_decoder_set_replace_msz_fullwidth_ascii(ctx->decoder,
> >>> -   
> >>> ctx->replace_fullwidth_a

Re: [FFmpeg-devel] [ANNOUNCE] upcoming GA vote

2023-10-27 Thread Jan Ekström
On Wed, Oct 25, 2023 at 4:14 PM Thilo Borgmann via ffmpeg-devel
 wrote:
>
> Hi,
>
> Am 24.10.23 um 23:15 schrieb Anton Khirnov:
> > Hi all,
> > as discussed at the dev meeting at VDD, we need to have a series of
> > votes, the first of which concerns defining when is the GA voter list to
> > be updated.
> >
> > As the previous attempt to vote on this was hampered by email delivery
> > issues, and also criticized due to inadequate advance announcement, the
> > vote was closed and we are going to try again.
> >
> > This time, to avoid any confusion, let me clearly state that this is a
> > declaration of intent to initiate a GA vote next Monday (2023-10-30),
> > unless there are substantial objections.
> >
> > The vote question will be:
> > How do we update the list of active members of the general assembly?
> >
> > Proposed possibilities so far are:
> > * twice a year (1st Jan & 1st July)
> >(suggested at VDD)
> > * before each vote
> >(suggested at VDD)
> > * never (keep the 2020 version)
> >(suggested at VDD)
> > * keep everyone who had vote rights but add active developers each jan/july
> >(suggested by Michael on the ML)
> >
> > Feel welcome to propose additional possibilities until Friday
> > 2023-10-27.
> >
> > Other constructive comments also welcome.
> >
>
> To test the voting beforehand this time, I created a test vote with a fake-GA
> member list. All of the following people should have received a corresponding
> mail. If you are part of this list, but did not receive an email, check your
> spam folder for the sender: ffmpeg.vot...@mail.de. Please comment if you 
> didn't
> get the mail.
>
> The list is as follows:
>
> andreas.rheinha...@gmail.com
> andriy.gel...@gmail.com
> an...@khirnov.net
> barryjz...@tencent.com
> ceffm...@gmail.com
> c...@passwd.hu
> dcni...@gmail.com
> derek.buitenh...@gmail.com
> d...@lynne.ee
> devin.heitmuel...@ltnglobal.com
> epira...@gmail.com
> ffm...@gyani.pro
> geo...@nsup.org
> g...@haasn.dev
> haihao.xi...@intel.com
> jamr...@gmail.com
> jan.ekst...@24i.com

I have received the e-mail this time, which is great. But it seems
like my worry about the mail map being the wrong way around was not
unfounded. This e-mail is not the "community member Jan" me, and
should never have any committer cases as I generally attempt to air
gap the two sides.

I'll try to post a patch today either inverting their order (which
hopefully outputs my community member e-mail), or - since this e-mail
is never supposed to apply to any committer cases - removing it.

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


Re: [FFmpeg-devel] [PATCH v4 1/3] lavc/libaribcaption.c: FIX: change all `boot` option var to `int`

2023-10-29 Thread Jan Ekström
On Tue, Oct 17, 2023 at 4:14 PM TADANO Tokumei  wrote:
>
> This patch fixes a bug in curret version.
>
> On some environments, a `bool` variable is small space than `int`.
> If a `bool` option was specified by command line, following
> variables would be filled and may be destroyed by av_opt_copy().
>
> This patch change all the `bool` option variables in
> `ARIBCaptionContext` to `int`.
>
> Signed-off-by: TADANO Tokumei 

Change LGTM. I changed the commit message to more match our guide
lines (no ".c" at the end, rephrased the commit message) in
https://github.com/jeeb/ffmpeg/commits/aribcaption_msz_patches_v3 .

Just tell me if you're OK with this and I'll pull it in.

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


Re: [FFmpeg-devel] [PATCH v4 2/3] lavc/libaribcaption.c: add MSZ characters related options

2023-10-29 Thread Jan Ekström
On Tue, Oct 17, 2023 at 4:14 PM TADANO Tokumei  wrote:
>
> This patch add MSZ (Middle Size; half width) characters related
> options.
>
> * add `-replace_msz_japanese` option introduced in version 1.0.1
>   of libaribcaption.
> * add `-replace_msz_glyph` option introduced in version 1.1.0
>   of libaribcaption.
>
> If specified fonts contain half-width glyphs (e.g., BIZ UDGothic),
> it make better rendering with `-replace_msz_ascii false` and
> `-replace_msz_japanese false` option for bitmap sub_type.
>
> Signed-off-by: TADANO Tokumei 

Change-wise LGTM. I reworked the commit message (starting with the
removal of ".c" at the end), and hopefully I understood correctly what
you were trying to mean with what I expect you were meaning with
regards to the latter option. The adjusted commits are visible in
https://github.com/jeeb/ffmpeg/commits/aribcaption_msz_patches_v3 .

Otherwise the only thing I adjusted was

diff --git a/libavcodec/libaribcaption.c b/libavcodec/libaribcaption.c
index 2a058a4992..e87f303aa8 100644
--- a/libavcodec/libaribcaption.c
+++ b/libavcodec/libaribcaption.c
@@ -1008,7 +1008,7 @@ static int aribcaption_init(AVCodecContext *avctx)
 aribcc_decoder_set_replace_msz_fullwidth_ascii(ctx->decoder,
ctx->replace_msz_ascii);
 aribcc_decoder_set_replace_msz_fullwidth_japanese(ctx->decoder,
-   ctx->replace_msz_japanese);
+
ctx->replace_msz_japanese);

 /* Similar behavior as ffmpeg tool to set canvas size */
 if (ctx->canvas_width > 0 && ctx->canvas_height > 0 &&

as there was a mismatching offset. If this is fine by you, I'll pull this in.

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


Re: [FFmpeg-devel] [PATCH v4 3/3] lavc/libaribcaption.c: rename `-replace_fullwidth_ascii` option

2023-10-29 Thread Jan Ekström
On Tue, Oct 17, 2023 at 4:14 PM TADANO Tokumei  wrote:
>
> This patch renames `-replace_fullwidth_ascii` option to
> `-replace_msz_ascii` to clarify option meaning.
>
> Signed-off-by: TADANO Tokumei 

Asked around and James seemed to be OK with just replacing this option
as this is a "niche" module and since it had not yet been in any
release. Thus LGTM change-wise.

Same things as with the rest of the set, did adjustments to the commit
message. If you're fine with it, I'll pull these in.

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


Re: [FFmpeg-devel] [PATCH v4 2/3] lavc/libaribcaption.c: add MSZ characters related options

2023-10-29 Thread Jan Ekström
On Sun, Oct 29, 2023 at 10:51 AM Jan Ekström  wrote:
>
> On Tue, Oct 17, 2023 at 4:14 PM TADANO Tokumei  wrote:
> >
> > This patch add MSZ (Middle Size; half width) characters related
> > options.
> >
> > * add `-replace_msz_japanese` option introduced in version 1.0.1
> >   of libaribcaption.
> > * add `-replace_msz_glyph` option introduced in version 1.1.0
> >   of libaribcaption.
> >
> > If specified fonts contain half-width glyphs (e.g., BIZ UDGothic),
> > it make better rendering with `-replace_msz_ascii false` and
> > `-replace_msz_japanese false` option for bitmap sub_type.
> >
> > Signed-off-by: TADANO Tokumei 
>
> Change-wise LGTM. I reworked the commit message (starting with the
> removal of ".c" at the end), and hopefully I understood correctly what
> you were trying to mean with what I expect you were meaning with
> regards to the latter option. The adjusted commits are visible in
> https://github.com/jeeb/ffmpeg/commits/aribcaption_msz_patches_v3 .
>
> Otherwise the only thing I adjusted was
>
> diff --git a/libavcodec/libaribcaption.c b/libavcodec/libaribcaption.c
> index 2a058a4992..e87f303aa8 100644
> --- a/libavcodec/libaribcaption.c
> +++ b/libavcodec/libaribcaption.c
> @@ -1008,7 +1008,7 @@ static int aribcaption_init(AVCodecContext *avctx)
>  aribcc_decoder_set_replace_msz_fullwidth_ascii(ctx->decoder,
> ctx->replace_msz_ascii);
>  aribcc_decoder_set_replace_msz_fullwidth_japanese(ctx->decoder,
> -   
> ctx->replace_msz_japanese);
> +
> ctx->replace_msz_japanese);
>
>  /* Similar behavior as ffmpeg tool to set canvas size */
>  if (ctx->canvas_width > 0 && ctx->canvas_height > 0 &&
>
> as there was a mismatching offset. If this is fine by you, I'll pull this in.

Received an OK from the author regarding the commit message
adjustments and such privately, so will be merging this in.
___
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] avfillter/buffersrc: activate and EOF fix

2023-11-01 Thread Jan Ekström
On Tue, Oct 31, 2023 at 12:55 PM Nicolas George  wrote:
>
> Paul B Mahol (12023-10-27):
> > Both patches are required to fix out of memory scenario with this use case:
>
> I checked. The OOM still happens with these two patches => they do not
> fix the issue => rejected.

So for me I have the following memory ballooning test case:

1. build FFmpeg with libx264 (I just haven't had the time to minimize
the test case to test other video formats)
2. Check that you have /usr/bin/time (it outputs "Maximum resident set
size" when called with -v argument)
3. Create test input with `ffmpeg -v verbose -filter_complex
'testsrc2=r=25:s=640x480:d=470[out]' -map '[out]' -c:v libx264 -preset
superfast test_clip.mp4`
4. Run the test case `/usr/bin/time -v ffmpeg -y -v verbose -i
test_clip.mp4 -c copy -map 0 -t 470 test_clip.mp4_copy.mp4
-filter_complex
"[0:0]split=1[thumb_in];[thumb_in]trim=start=420:end=421,scale=720:-2:threads=1,setsar=1/1,hqdn3d,unsharp[thumb_out]"
-map "[thumb_out]" -vframes 1 -f image2 test_clip.mp4_img2.jpg`

If the result for this is a maximum resident set size of about 60MiB
(f.ex. Maximum resident set size (kbytes): 62752), then things are how
they were before trim was switched to activate. If it is closer to
500MiB+ (f.ex. Maximum resident set size (kbytes): 609744), then that
is the memory ballooning behavior in action.

Results:

For current master the result is: bad (over 500MiB and would have kept
growing without the time limiting)
After applying the two patches: good (back to closer to 50MiB than 500MiB)

Exact steps to apply patches:
curl -L 
"https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20231027/8bf21777/attachment.bin";
| git am
curl -L 
"https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20231027/8bf21777/attachment-0001.bin";
| git am

I was planning to make a ticket for this, but Paul was too quick to
fix this so there is no trac ticket yet.

So my question is: Does this test case not improve for you after you
have applied these patches? Or are you speaking of a separate problem
which is bad both in master as well as after these patches have been
applied?

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


Re: [FFmpeg-devel] [PATCH] ffmpeg CLI multithreading

2023-11-13 Thread Jan Ekström
On Sat, Nov 4, 2023 at 11:22 AM Anton Khirnov  wrote:
>
> Hi,
> after ~2 years of work and ~700+ preparatory commits, here is finally
> the first "fully functional" version of multithreaded ffmpeg CLI. In
> quotes because due to the scale of the changes I'm sure some things got
> broken and I didn't notice - more testing very much welcome.
>
> One thing which is most definitely broken is the
> -fix_sub_duration_heartbeat option, which in its current form makes
> assumptions about synchronization between distant components
> (encoders/muxers -> decoders), which makes it fundamentally
> non-deterministic when each of these components runs asynchronously
> (note that its behaviour is unpredictable even now, it's just
> deterministic across runs with the same options). I'm currently
> disabling the option in 16/24, better suggestions on what to do with it
> are welcome.

Tested a version of this branch without the fix_sub_duration_heartbeat
removal 
(https://github.com/jeeb/ffmpeg/commits/ffmpeg_threading_with_sub_heartbeat),
and was expecting the functionality to be completely
non-deterministic. Yet to my surprise a single GEN=1 update worked on
all of the x86 devices from Thinkpad x230 to a gen3 Ryzen laptop. The
threading seems to allow for the output to fall behind by some amount
(in this case about ~200ms?), but the FATE test seems to be very
deterministic with these components on the systems I could test and
with THREADS set to 16/32/128, putting a load on the system etc. As a
test I also switched the encoder to libx264, preset placebo and the
only thing that affected was the amount of delay, but the test was
still seemingly deterministic.

I would request people to test
"fate-ffmpeg-fix_sub_duration_heartbeat" on something a bit more
exotic from this branch, just to see if the same result is attained.
If it is, then this specific test case would seem to be stable stable
and not blocking the threading work.

As for output and input being not in synch, for this feature it should
be completely fine, since the range is from "at the point of input" to
"input has gotten all the way to EOF while output is playing
catch-up", which just means that there is more and more information
available when writing out the output, and the base value is always
the point of input (you should (?) never be in a state where output is
further than the input, and thus you are missing information from the
input from that specific point of time).

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] [PATCH v5 00/14] encoder AVCodecContext configuration side data

2023-11-26 Thread Jan Ekström
Differences to v3:
1. rebased on top of current master
2. moved the addition of multiple side data entries from a generic
   av_frame_side_data_set_extend to avcodec as per request from James.
4. adopted various things noted by reviews

Comparison URL (mostly configure and wrappers, avutil/frame.c):
https://github.com/jeeb/ffmpeg/compare/avcodec_cll_mdcv_side_data_v4..avcodec_cll_mdcv_side_data_v5

This patch set I've now been working for a while since I felt like it was weird
we couldn't pass through information such as static HDR metadata to encoders
from decoded input. This initial version adds the necessary framework, as well
as adds static HDR metadata support for libsvtav1, libx264 as well as libx265
wrappers.

An alternative to this would be to make encoders only properly initialize when
they receive the first AVFrame, but that seems to be a bigger, nastier change
than introducing an AVFrameSideDataSet in avctx as everything seems to
presume that extradata etc are available after opening the encoder.

Note: Any opinions on whether FFCodec or AVCodec should have
  handled_side_data list, so that if format specific generic logic is
  added, it could be checked whether the codec itself handles this side
  data? This could also be utilized to list handled side data from f.ex.
  `ffmpeg -h encoder=libsvtav1`.

Jan


Jan Ekström (14):
  avutil/frame: add AVFrameSideDataSet for passing sets of side data
  avutil/frame: split side data list wiping out to non-AVFrame function
  avutil/frame: add helper for uninitializing side data sets
  avutil/frame: split side_data_from_buf to base and AVFrame func
  avutil/frame: split side data removal out to non-AVFrame function
  avutil/frame: add helper for adding side data to set
  avutil/frame: add helper for adding existing side data to set
  avutil/frame: add helper for getting side data from set
  avcodec: add side data set to AVCodecContext
  avcodec: add helper for configuring AVCodecContext's side data set
  ffmpeg: pass first video AVFrame's side data to encoder
  avcodec/libsvtav1: add support for writing out CLL and MDCV
  avcodec/libx264: add support for writing out CLL and MDCV
  avcodec/libx265: add support for writing out CLL and MDCV

 configure   |   2 +
 fftools/ffmpeg_enc.c|  13 +++
 libavcodec/avcodec.c|  28 ++
 libavcodec/avcodec.h|  27 +
 libavcodec/libsvtav1.c  |  67 +
 libavcodec/libx264.c|  78 ++
 libavcodec/libx265.c|  87 
 libavcodec/options.c|   1 +
 libavutil/Makefile  |   1 +
 libavutil/frame.c   | 173 ++--
 libavutil/frame.h   |  64 
 libavutil/tests/side_data_set.c |  97 ++
 tests/fate/enc_external.mak |  15 +++
 tests/fate/libavutil.mak|   4 +
 tests/ref/fate/libsvtav1-hdr10  |  14 +++
 tests/ref/fate/libx264-hdr10|  15 +++
 tests/ref/fate/libx265-hdr10|  16 +++
 tests/ref/fate/side_data_set|  14 +++
 18 files changed, 686 insertions(+), 30 deletions(-)
 create mode 100644 libavutil/tests/side_data_set.c
 create mode 100644 tests/ref/fate/libsvtav1-hdr10
 create mode 100644 tests/ref/fate/libx264-hdr10
 create mode 100644 tests/ref/fate/libx265-hdr10
 create mode 100644 tests/ref/fate/side_data_set

-- 
2.43.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 v5 01/14] avutil/frame: add AVFrameSideDataSet for passing sets of side data

2023-11-26 Thread Jan Ekström
---
 libavutil/frame.h | 8 
 1 file changed, 8 insertions(+)

diff --git a/libavutil/frame.h b/libavutil/frame.h
index c0c1b23db7..6155226c1d 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -251,6 +251,14 @@ typedef struct AVFrameSideData {
 AVBufferRef *buf;
 } AVFrameSideData;
 
+/**
+ * Structure to hold a set of AVFrameSideData
+ */
+typedef struct AVFrameSideDataSet {
+AVFrameSideData **sd;
+intnb_sd;
+} AVFrameSideDataSet;
+
 /**
  * Structure describing a single Region Of Interest.
  *
-- 
2.43.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 v5 02/14] avutil/frame: split side data list wiping out to non-AVFrame function

2023-11-26 Thread Jan Ekström
This will make it possible to to reuse logic in further commits.
---
 libavutil/frame.c | 23 ++-
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index a3f07ca089..d94710687b 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -75,14 +75,19 @@ static void free_side_data(AVFrameSideData **ptr_sd)
 av_freep(ptr_sd);
 }
 
-static void wipe_side_data(AVFrame *frame)
+static void wipe_side_data(AVFrameSideData ***sd, int *nb_side_data)
 {
-for (int i = 0; i < frame->nb_side_data; i++) {
-free_side_data(&frame->side_data[i]);
+for (int i = 0; i < *nb_side_data; i++) {
+free_side_data(&((*sd)[i]));
 }
-frame->nb_side_data = 0;
+*nb_side_data = 0;
+
+av_freep(sd);
+}
 
-av_freep(&frame->side_data);
+static void frame_side_data_wipe(AVFrame *frame)
+{
+wipe_side_data(&frame->side_data, &frame->nb_side_data);
 }
 
 AVFrame *av_frame_alloc(void)
@@ -337,7 +342,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
 sd_dst = av_frame_new_side_data(dst, sd_src->type,
 sd_src->size);
 if (!sd_dst) {
-wipe_side_data(dst);
+frame_side_data_wipe(dst);
 return AVERROR(ENOMEM);
 }
 memcpy(sd_dst->data, sd_src->data, sd_src->size);
@@ -346,7 +351,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
 sd_dst = av_frame_new_side_data_from_buf(dst, sd_src->type, ref);
 if (!sd_dst) {
 av_buffer_unref(&ref);
-wipe_side_data(dst);
+frame_side_data_wipe(dst);
 return AVERROR(ENOMEM);
 }
 }
@@ -525,7 +530,7 @@ FF_DISABLE_DEPRECATION_WARNINGS
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
 
-wipe_side_data(dst);
+frame_side_data_wipe(dst);
 av_dict_free(&dst->metadata);
 ret = frame_copy_props(dst, src, 0);
 if (ret < 0)
@@ -624,7 +629,7 @@ void av_frame_unref(AVFrame *frame)
 if (!frame)
 return;
 
-wipe_side_data(frame);
+frame_side_data_wipe(frame);
 
 for (int i = 0; i < FF_ARRAY_ELEMS(frame->buf); i++)
 av_buffer_unref(&frame->buf[i]);
-- 
2.43.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 v5 03/14] avutil/frame: add helper for uninitializing side data sets

2023-11-26 Thread Jan Ekström
---
 libavutil/frame.c | 5 +
 libavutil/frame.h | 8 
 2 files changed, 13 insertions(+)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index d94710687b..941a0a8148 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -90,6 +90,11 @@ static void frame_side_data_wipe(AVFrame *frame)
 wipe_side_data(&frame->side_data, &frame->nb_side_data);
 }
 
+void av_frame_side_data_set_uninit(AVFrameSideDataSet *set)
+{
+wipe_side_data(&set->sd, &set->nb_sd);
+}
+
 AVFrame *av_frame_alloc(void)
 {
 AVFrame *frame = av_malloc(sizeof(*frame));
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 6155226c1d..4fe9ac9411 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -1057,6 +1057,14 @@ int av_frame_apply_cropping(AVFrame *frame, int flags);
  */
 const char *av_frame_side_data_name(enum AVFrameSideDataType type);
 
+/**
+ * Free all side data entries and their contents, then zeroes out the
+ * struct values.
+ *
+ * @param set the set which should be uninitialized
+ */
+void av_frame_side_data_set_uninit(AVFrameSideDataSet *set);
+
 /**
  * @}
  */
-- 
2.43.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 v5 04/14] avutil/frame: split side_data_from_buf to base and AVFrame func

2023-11-26 Thread Jan Ekström
---
 libavutil/frame.c | 31 +++
 1 file changed, 23 insertions(+), 8 deletions(-)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index 941a0a8148..49da452fa5 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -787,23 +787,22 @@ FF_ENABLE_DEPRECATION_WARNINGS
 return NULL;
 }
 
-AVFrameSideData *av_frame_new_side_data_from_buf(AVFrame *frame,
- enum AVFrameSideDataType type,
- AVBufferRef *buf)
+static AVFrameSideData *add_side_data_to_set_from_buf(AVFrameSideDataSet *set,
+  enum AVFrameSideDataType 
type,
+  AVBufferRef *buf)
 {
 AVFrameSideData *ret, **tmp;
 
 if (!buf)
 return NULL;
 
-if (frame->nb_side_data > INT_MAX / sizeof(*frame->side_data) - 1)
+if (set->nb_sd > INT_MAX / sizeof(*set->sd) - 1)
 return NULL;
 
-tmp = av_realloc(frame->side_data,
- (frame->nb_side_data + 1) * sizeof(*frame->side_data));
+tmp = av_realloc(set->sd, (set->nb_sd + 1) * sizeof(*set->sd));
 if (!tmp)
 return NULL;
-frame->side_data = tmp;
+set->sd = tmp;
 
 ret = av_mallocz(sizeof(*ret));
 if (!ret)
@@ -814,7 +813,23 @@ AVFrameSideData *av_frame_new_side_data_from_buf(AVFrame 
*frame,
 ret->size = buf->size;
 ret->type = type;
 
-frame->side_data[frame->nb_side_data++] = ret;
+set->sd[set->nb_sd++] = ret;
+
+return ret;
+}
+
+AVFrameSideData *av_frame_new_side_data_from_buf(AVFrame *frame,
+ enum AVFrameSideDataType type,
+ AVBufferRef *buf)
+{
+AVFrameSideDataSet set = {
+.sd= frame->side_data,
+.nb_sd = frame->nb_side_data,
+};
+AVFrameSideData *ret = add_side_data_to_set_from_buf(&set, type, buf);
+
+frame->side_data= set.sd;
+frame->nb_side_data = set.nb_sd;
 
 return ret;
 }
-- 
2.43.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 v5 05/14] avutil/frame: split side data removal out to non-AVFrame function

2023-11-26 Thread Jan Ekström
This will make it possible to reuse logic in further commits.
---
 libavutil/frame.c | 24 
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index 49da452fa5..587e212d1a 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -95,6 +95,21 @@ void av_frame_side_data_set_uninit(AVFrameSideDataSet *set)
 wipe_side_data(&set->sd, &set->nb_sd);
 }
 
+static void remove_side_data(AVFrameSideData ***sd, int *nb_side_data,
+ const enum AVFrameSideDataType type)
+{
+for (int i = *nb_side_data - 1; i >= 0; i--) {
+AVFrameSideData *entry = ((*sd)[i]);
+if (entry->type != type)
+continue;
+
+free_side_data(&entry);
+
+((*sd)[i]) = ((*sd)[*nb_side_data - 1]);
+(*nb_side_data)--;
+}
+}
+
 AVFrame *av_frame_alloc(void)
 {
 AVFrame *frame = av_malloc(sizeof(*frame));
@@ -943,14 +958,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
 
 void av_frame_remove_side_data(AVFrame *frame, enum AVFrameSideDataType type)
 {
-for (int i = frame->nb_side_data - 1; i >= 0; i--) {
-AVFrameSideData *sd = frame->side_data[i];
-if (sd->type == type) {
-free_side_data(&frame->side_data[i]);
-frame->side_data[i] = frame->side_data[frame->nb_side_data - 1];
-frame->nb_side_data--;
-}
-}
+remove_side_data(&frame->side_data, &frame->nb_side_data, type);
 }
 
 const char *av_frame_side_data_name(enum AVFrameSideDataType type)
-- 
2.43.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 v5 06/14] avutil/frame: add helper for adding side data to set

2023-11-26 Thread Jan Ekström
Additionally, add an API test to check that the no-duplicates
addition works after duplicates have been inserted.
---
 libavutil/Makefile  |  1 +
 libavutil/frame.c   | 18 ++
 libavutil/frame.h   | 20 +++
 libavutil/tests/side_data_set.c | 97 +
 tests/fate/libavutil.mak|  4 ++
 tests/ref/fate/side_data_set| 14 +
 6 files changed, 154 insertions(+)
 create mode 100644 libavutil/tests/side_data_set.c
 create mode 100644 tests/ref/fate/side_data_set

diff --git a/libavutil/Makefile b/libavutil/Makefile
index 4711f8cde8..c000fa0c7e 100644
--- a/libavutil/Makefile
+++ b/libavutil/Makefile
@@ -266,6 +266,7 @@ TESTPROGS = adler32 
\
 ripemd  \
 sha \
 sha512  \
+side_data_set   \
 softfloat   \
 tree\
 twofish \
diff --git a/libavutil/frame.c b/libavutil/frame.c
index 587e212d1a..9ac3db430f 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -861,6 +861,24 @@ AVFrameSideData *av_frame_new_side_data(AVFrame *frame,
 return ret;
 }
 
+AVFrameSideData *av_frame_side_data_set_new_entry(AVFrameSideDataSet *set,
+  enum AVFrameSideDataType 
type,
+  size_t size,
+  unsigned int flags)
+{
+AVBufferRef *buf = av_buffer_alloc(size);
+AVFrameSideData *ret = NULL;
+
+if (flags & AV_FRAME_SIDE_DATA_SET_FLAG_NO_DUPLICATES)
+remove_side_data(&set->sd, &set->nb_sd, type);
+
+ret = add_side_data_to_set_from_buf(set, type, buf);
+if (!ret)
+av_buffer_unref(&buf);
+
+return ret;
+}
+
 AVFrameSideData *av_frame_get_side_data(const AVFrame *frame,
 enum AVFrameSideDataType type)
 {
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 4fe9ac9411..093a76e30d 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -1065,6 +1065,26 @@ const char *av_frame_side_data_name(enum 
AVFrameSideDataType type);
  */
 void av_frame_side_data_set_uninit(AVFrameSideDataSet *set);
 
+#define AV_FRAME_SIDE_DATA_SET_FLAG_NO_DUPLICATES (1 << 0)
+
+/**
+ * Add a new side data entry to a set.
+ *
+ * @param set a set to which the side data should be added
+ * @param type type of the added side data
+ * @param size size of the side data
+ * @param flags Some combination of AV_FRAME_SIDE_DATA_SET_FLAG_* flags, or 0.
+ *
+ * @return newly added side data on success, NULL on error. In case of
+ * AV_FRAME_SIDE_DATA_SET_FLAG_NO_DUPLICATES being set, entries
+ * of matching AVFrameSideDataType will be removed before the
+ * addition is attempted.
+ */
+AVFrameSideData *av_frame_side_data_set_new_entry(AVFrameSideDataSet *set,
+  enum AVFrameSideDataType 
type,
+  size_t size,
+  unsigned int flags);
+
 /**
  * @}
  */
diff --git a/libavutil/tests/side_data_set.c b/libavutil/tests/side_data_set.c
new file mode 100644
index 00..820b3aac44
--- /dev/null
+++ b/libavutil/tests/side_data_set.c
@@ -0,0 +1,97 @@
+/*
+ * Copyright (c) 2023 Jan Ekström 
+ *
+ * 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 "libavutil/frame.c"
+#include "libavutil/mastering_display_metadata.h"
+
+static void print_clls(const AVFrameSideDataSet set)
+{
+for (int i = 0; i < set.nb_sd; i++) {
+AVFrameSideData *sd = set.sd[i];
+
+printf("sd %d, %s",
+   i, av_frame_side_data_name(sd->type));
+
+if (sd->type != AV_FRAME_DATA_CONTENT_LIGHT_LEVEL) {
+   

[FFmpeg-devel] [PATCH v5 07/14] avutil/frame: add helper for adding existing side data to set

2023-11-26 Thread Jan Ekström
---
 libavutil/frame.c | 50 +++
 libavutil/frame.h | 16 +++
 2 files changed, 66 insertions(+)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index 9ac3db430f..e42d9cb758 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -110,6 +110,24 @@ static void remove_side_data(AVFrameSideData ***sd, int 
*nb_side_data,
 }
 }
 
+static void remove_side_data_by_entry(AVFrameSideData ***sd,
+  int *nb_side_data,
+  const AVFrameSideData *target)
+{
+for (int i = *nb_side_data - 1; i >= 0; i--) {
+AVFrameSideData *entry = ((*sd)[i]);
+if (entry != target)
+continue;
+
+free_side_data(&entry);
+
+((*sd)[i]) = ((*sd)[*nb_side_data - 1]);
+(*nb_side_data)--;
+
+return;
+}
+}
+
 AVFrame *av_frame_alloc(void)
 {
 AVFrame *frame = av_malloc(sizeof(*frame));
@@ -879,6 +897,38 @@ AVFrameSideData 
*av_frame_side_data_set_new_entry(AVFrameSideDataSet *set,
 return ret;
 }
 
+int av_frame_side_data_set_entry_from_sd(AVFrameSideDataSet *dst,
+ const AVFrameSideData *src,
+ unsigned int flags)
+{
+if (!dst || !src)
+return AVERROR(EINVAL);
+
+{
+AVBufferRef   *buf= av_buffer_ref(src->buf);
+AVFrameSideData   *sd_dst = NULL;
+
+if (flags & AV_FRAME_SIDE_DATA_SET_FLAG_NO_DUPLICATES)
+remove_side_data(&dst->sd, &dst->nb_sd, src->type);
+
+sd_dst = add_side_data_to_set_from_buf(dst, src->type, buf);
+if (!sd_dst) {
+av_buffer_unref(&buf);
+return AVERROR(ENOMEM);
+}
+
+{
+int ret = av_dict_copy(&sd_dst->metadata, src->metadata, 0);
+if (ret < 0) {
+remove_side_data_by_entry(&dst->sd, &dst->nb_sd, sd_dst);
+return ret;
+}
+}
+
+return 0;
+}
+}
+
 AVFrameSideData *av_frame_get_side_data(const AVFrame *frame,
 enum AVFrameSideDataType type)
 {
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 093a76e30d..9295c868ef 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -1085,6 +1085,22 @@ AVFrameSideData 
*av_frame_side_data_set_new_entry(AVFrameSideDataSet *set,
   size_t size,
   unsigned int flags);
 
+/**
+ * Add a new side data entry to a set based on existing side data.
+ *
+ * @param dst a set to which the side data should be added
+ * @param src side data which should be added to the set
+ * @param flags Some combination of AV_FRAME_SIDE_DATA_SET_FLAG_* flags, or 0.
+ *
+ * @return negative error code on failure, >=0 on success. In case of
+ * AV_FRAME_SIDE_DATA_SET_FLAG_NO_DUPLICATES being set, entries
+ * of matching AVFrameSideDataType will be removed before the
+ * addition is attempted.
+ */
+int av_frame_side_data_set_entry_from_sd(AVFrameSideDataSet *dst,
+ const AVFrameSideData *src,
+ unsigned int flags);
+
 /**
  * @}
  */
-- 
2.43.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 v5 08/14] avutil/frame: add helper for getting side data from set

2023-11-26 Thread Jan Ekström
---
 libavutil/frame.c | 22 +-
 libavutil/frame.h | 12 
 2 files changed, 29 insertions(+), 5 deletions(-)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index e42d9cb758..e4004daa4b 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -929,16 +929,28 @@ int 
av_frame_side_data_set_entry_from_sd(AVFrameSideDataSet *dst,
 }
 }
 
-AVFrameSideData *av_frame_get_side_data(const AVFrame *frame,
-enum AVFrameSideDataType type)
+AVFrameSideData *av_frame_side_data_set_get_entry(const AVFrameSideDataSet set,
+  enum AVFrameSideDataType 
type)
 {
-for (int i = 0; i < frame->nb_side_data; i++) {
-if (frame->side_data[i]->type == type)
-return frame->side_data[i];
+for (int i = 0; i < set.nb_sd; i++) {
+if (set.sd[i]->type == type)
+return set.sd[i];
 }
 return NULL;
 }
 
+AVFrameSideData *av_frame_get_side_data(const AVFrame *frame,
+enum AVFrameSideDataType type)
+{
+return av_frame_side_data_set_get_entry(
+(const AVFrameSideDataSet){
+.sd= frame->side_data,
+.nb_sd = frame->nb_side_data
+},
+type
+);
+}
+
 static int frame_copy_video(AVFrame *dst, const AVFrame *src)
 {
 int planes;
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 9295c868ef..e8517bf6ad 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -1101,6 +1101,18 @@ int 
av_frame_side_data_set_entry_from_sd(AVFrameSideDataSet *dst,
  const AVFrameSideData *src,
  unsigned int flags);
 
+/**
+ * Get a side data entry of a specific type from a set.
+ *
+ * @param set the set from which side data should be queried from
+ * @param type type of side data to be queried
+ *
+ * @return a pointer to the side data of a given type on success, NULL if there
+ * is no side data with such type in this set.
+ */
+AVFrameSideData *av_frame_side_data_set_get_entry(const AVFrameSideDataSet set,
+  enum AVFrameSideDataType 
type);
+
 /**
  * @}
  */
-- 
2.43.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 v5 09/14] avcodec: add side data set to AVCodecContext

2023-11-26 Thread Jan Ekström
This allows configuring an encoder by using AVFrameSideData.
---
 libavcodec/avcodec.h | 7 +++
 libavcodec/options.c | 1 +
 2 files changed, 8 insertions(+)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 7fb44e28f4..0c1a8ade49 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -2116,6 +2116,13 @@ typedef struct AVCodecContext {
  *   an error.
  */
 int64_t frame_num;
+
+/**
+ * Set containing static side data, such as HDR10 CLL / MDCV structures.
+ * - encoding: set by user
+ * - decoding: unused
+ */
+AVFrameSideDataSet frame_sd_set;
 } AVCodecContext;
 
 /**
diff --git a/libavcodec/options.c b/libavcodec/options.c
index a9b35ee1c3..e42a29e834 100644
--- a/libavcodec/options.c
+++ b/libavcodec/options.c
@@ -180,6 +180,7 @@ void avcodec_free_context(AVCodecContext **pavctx)
 av_freep(&avctx->inter_matrix);
 av_freep(&avctx->rc_override);
 av_channel_layout_uninit(&avctx->ch_layout);
+av_frame_side_data_set_uninit(&avctx->frame_sd_set);
 
 av_freep(pavctx);
 }
-- 
2.43.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 v5 10/14] avcodec: add helper for configuring AVCodecContext's side data set

2023-11-26 Thread Jan Ekström
This allows API clients that wish to configure multiple entries
at a time to do so without writing the looping code themselves.
---
 libavcodec/avcodec.c | 28 
 libavcodec/avcodec.h | 20 
 2 files changed, 48 insertions(+)

diff --git a/libavcodec/avcodec.c b/libavcodec/avcodec.c
index 2dda310e91..9b56af8114 100644
--- a/libavcodec/avcodec.c
+++ b/libavcodec/avcodec.c
@@ -718,3 +718,31 @@ int attribute_align_arg 
avcodec_receive_frame(AVCodecContext *avctx, AVFrame *fr
 return ff_decode_receive_frame(avctx, frame);
 return ff_encode_receive_frame(avctx, frame);
 }
+
+int avcodec_configure_side_data(AVCodecContext *avctx,
+const AVFrameSideDataSet *set,
+unsigned int flags)
+{
+if (!avctx)
+return AVERROR(EINVAL);
+
+if (!set) {
+av_frame_side_data_set_uninit(&avctx->frame_sd_set);
+return 0;
+}
+
+if (set->nb_sd > 0 && set->nb_sd == avctx->frame_sd_set.nb_sd &&
+set->sd == avctx->frame_sd_set.sd)
+return AVERROR(EINVAL);
+
+for (int i = 0; i < set->nb_sd; i++) {
+int ret = av_frame_side_data_set_entry_from_sd(
+&avctx->frame_sd_set, set->sd[i], flags);
+if (ret < 0) {
+av_frame_side_data_set_uninit(&avctx->frame_sd_set);
+return ret;
+}
+}
+
+return 0;
+}
diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 0c1a8ade49..40261594f1 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -3154,6 +3154,26 @@ void av_fast_padded_mallocz(void *ptr, unsigned int 
*size, size_t min_size);
  */
 int avcodec_is_open(AVCodecContext *s);
 
+/**
+ * Add multiple side data entries to an AVCodecContext set in one go, for
+ * example from an AVFrame.
+ *
+ * In case the function fails to add a side data entry, it will clear the
+ * whole side data set.
+ *
+ * @param avctx context to which the side data should be added
+ * @param set a set from which the side data should be added from.
+ *if null, clears out the side data for this context.
+ * @param flags Some combination of AV_FRAME_SIDE_DATA_SET_FLAG_* flags, or 0.
+ *
+ * @return negative error code on failure, >=0 on success.
+ *
+ * @see av_frame_side_data_set_new_entry regarding the flags.
+ */
+int avcodec_configure_side_data(AVCodecContext *avctx,
+const AVFrameSideDataSet *set,
+unsigned int flags);
+
 /**
  * @}
  */
-- 
2.43.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 v5 11/14] ffmpeg: pass first video AVFrame's side data to encoder

2023-11-26 Thread Jan Ekström
This enables further configuration of output based on the results
of input decoding and filtering in a similar manner as the color
information.
---
 fftools/ffmpeg_enc.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/fftools/ffmpeg_enc.c b/fftools/ffmpeg_enc.c
index fa4539664f..8ec9cb7cd5 100644
--- a/fftools/ffmpeg_enc.c
+++ b/fftools/ffmpeg_enc.c
@@ -239,6 +239,19 @@ int enc_open(OutputStream *ost, const AVFrame *frame)
 enc_ctx->colorspace = frame->colorspace;
 enc_ctx->chroma_sample_location = frame->chroma_location;
 
+ret = avcodec_configure_side_data(
+enc_ctx,
+&(const AVFrameSideDataSet){
+.sd= frame->side_data,
+.nb_sd = frame->nb_side_data
+},
+AV_FRAME_SIDE_DATA_SET_FLAG_NO_DUPLICATES);
+if (ret < 0) {
+av_log(NULL, AV_LOG_ERROR, "failed to configure video encoder: 
%s!\n",
+   av_err2str(ret));
+return ret;
+}
+
 if (enc_ctx->flags & (AV_CODEC_FLAG_INTERLACED_DCT | 
AV_CODEC_FLAG_INTERLACED_ME) ||
 (frame->flags & AV_FRAME_FLAG_INTERLACED)
 #if FFMPEG_OPT_TOP
-- 
2.43.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 v5 12/14] avcodec/libsvtav1: add support for writing out CLL and MDCV

2023-11-26 Thread Jan Ekström
These two were added in 28e23d7f348c78d49a726c7469f9d4e38edec341
and 3558c1f2e97455e0b89edef31b9a72ab7fa30550 for version 0.9.0 of
SVT-AV1, which is also our minimum requirement right now.

In other words, no additional version limiting conditions seem
to be required.

Additionally, add a FATE test which verifies that pass-through of
the MDCV/CLL side data is working during encoding.
---
 libavcodec/libsvtav1.c | 67 ++
 tests/fate/enc_external.mak|  5 +++
 tests/ref/fate/libsvtav1-hdr10 | 14 +++
 3 files changed, 86 insertions(+)
 create mode 100644 tests/ref/fate/libsvtav1-hdr10

diff --git a/libavcodec/libsvtav1.c b/libavcodec/libsvtav1.c
index 862192945b..ee75570c00 100644
--- a/libavcodec/libsvtav1.c
+++ b/libavcodec/libsvtav1.c
@@ -27,6 +27,8 @@
 #include "libavutil/common.h"
 #include "libavutil/frame.h"
 #include "libavutil/imgutils.h"
+#include "libavutil/intreadwrite.h"
+#include "libavutil/mastering_display_metadata.h"
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
 #include "libavutil/avassert.h"
@@ -146,6 +148,69 @@ static int alloc_buffer(EbSvtAv1EncConfiguration *config, 
SvtContext *svt_enc)
 
 }
 
+static void handle_mdcv(struct EbSvtAv1MasteringDisplayInfo *dst,
+const AVMasteringDisplayMetadata *mdcv)
+{
+if (mdcv->has_primaries) {
+const struct EbSvtAv1ChromaPoints *const points[] = {
+&dst->r,
+&dst->g,
+&dst->b,
+};
+
+for (int i = 0; i < 3; i++) {
+const struct EbSvtAv1ChromaPoints *dst = points[i];
+const AVRational *src = mdcv->display_primaries[i];
+
+AV_WB16(&dst->x,
+av_rescale_q(1, src[0], (AVRational){ 1, (1 << 16) }));
+AV_WB16(&dst->y,
+av_rescale_q(1, src[1], (AVRational){ 1, (1 << 16) }));
+}
+
+AV_WB16(&dst->white_point.x,
+av_rescale_q(1, mdcv->white_point[0],
+ (AVRational){ 1, (1 << 16) }));
+AV_WB16(&dst->white_point.y,
+av_rescale_q(1, mdcv->white_point[1],
+ (AVRational){ 1, (1 << 16) }));
+}
+
+if (mdcv->has_luminance) {
+AV_WB32(&dst->max_luma,
+av_rescale_q(1, mdcv->max_luminance,
+ (AVRational){ 1, (1 << 8) }));
+AV_WB32(&dst->min_luma,
+av_rescale_q(1, mdcv->min_luminance,
+ (AVRational){ 1, (1 << 14) }));
+}
+}
+
+static void handle_side_data(AVCodecContext *avctx,
+ EbSvtAv1EncConfiguration *param)
+{
+const AVFrameSideDataSet set = avctx->frame_sd_set;
+const AVFrameSideData *cll_sd =
+av_frame_side_data_set_get_entry(
+set, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
+const AVFrameSideData *mdcv_sd =
+av_frame_side_data_set_get_entry(
+set, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
+
+if (cll_sd) {
+const AVContentLightMetadata *cll =
+(AVContentLightMetadata *)cll_sd->data;
+
+AV_WB16(¶m->content_light_level.max_cll, cll->MaxCLL);
+AV_WB16(¶m->content_light_level.max_fall, cll->MaxFALL);
+}
+
+if (mdcv_sd) {
+handle_mdcv(¶m->mastering_display,
+(AVMasteringDisplayMetadata *)mdcv_sd->data);
+}
+}
+
 static int config_enc_params(EbSvtAv1EncConfiguration *param,
  AVCodecContext *avctx)
 {
@@ -280,6 +345,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
 /* 2 = IDR, closed GOP, 1 = CRA, open GOP */
 param->intra_refresh_type = avctx->flags & AV_CODEC_FLAG_CLOSED_GOP ? 2 : 
1;
 
+handle_side_data(avctx, param);
+
 #if SVT_AV1_CHECK_VERSION(0, 9, 1)
 while ((en = av_dict_get(svt_enc->svtav1_opts, "", en, 
AV_DICT_IGNORE_SUFFIX))) {
 EbErrorType ret = svt_av1_enc_parse_parameter(param, en->key, 
en->value);
diff --git a/tests/fate/enc_external.mak b/tests/fate/enc_external.mak
index 7eabebcc51..d787941c16 100644
--- a/tests/fate/enc_external.mak
+++ b/tests/fate/enc_external.mak
@@ -2,5 +2,10 @@ FATE_ENC_EXTERNAL-$(call ENCDEC, LIBX264 H264, MOV, 
H264_DEMUXER) += fate-libx26
 fate-libx264-simple: CMD = enc_external 
$(TARGET_SAMPLES)/h264-conformance/BA1_Sony_D.jsv \
 mp4 "-c:v libx264" "-show_entries frame=width,height,pix_fmt,pts,pkt_dts 
-of flat"
 
+# test for SVT-AV1 MDCV and CLL passthrough during encoding
+FATE_ENC_EXTERNAL-$(call ENCDEC, LIBSVTAV1 HEVC, MOV, HEVC_DEMUXER 
LIBDAV1D_DECODER) += fate-libsvtav1-hdr10
+fate-libsvtav1-hdr10: CMD = enc_external 
$(TARGET_SAMPLES)/hevc/hdr10_plus_h265_sample.hevc \
+mp4 "-c:v libsvtav1" "-show_frames -show_entries frame=side_data_list -of 
flat"
+
 FATE_SAMPLES_FFMPEG_FFPROBE += $(FATE_ENC_EXTERNAL-yes)
 fate-enc-external: $(FATE_ENC_EXTERNAL-yes)
diff --git a/tests/ref/fate/libsvtav1-hdr10 b/tests/ref/fate/libsvtav1-hdr10
new file mode 100644
index 00.

[FFmpeg-devel] [PATCH v5 13/14] avcodec/libx264: add support for writing out CLL and MDCV

2023-11-26 Thread Jan Ekström
Both of these two structures were first available with X264_BUILD
163, so make relevant functionality conditional on the version
being at least such.

Keep handle_side_data available in all cases as this way X264_init
does not require additional version based conditions within it.

Finally, add a FATE test which verifies that pass-through of the
MDCV/CLL side data is working during encoding.
---
 configure|  2 +
 libavcodec/libx264.c | 78 
 tests/fate/enc_external.mak  |  5 +++
 tests/ref/fate/libx264-hdr10 | 15 +++
 4 files changed, 100 insertions(+)
 create mode 100644 tests/ref/fate/libx264-hdr10

diff --git a/configure b/configure
index 838e627084..1b51fa7767 100755
--- a/configure
+++ b/configure
@@ -2514,6 +2514,7 @@ CONFIG_EXTRA="
 jpegtables
 lgplv3
 libx262
+libx264_hdr10
 llauddsp
 llviddsp
 llvidencdsp
@@ -6873,6 +6874,7 @@ enabled libx264   && require_pkg_config libx264 
x264 "stdint.h x264.h" x
  require_cpp_condition libx264 x264.h "X264_BUILD 
>= 122" && {
  [ "$toolchain" != "msvc" ] ||
  require_cpp_condition libx264 x264.h "X264_BUILD 
>= 158"; } &&
+ check_cpp_condition libx264_hdr10 x264.h 
"X264_BUILD >= 163" &&
  check_cpp_condition libx262 x264.h "X264_MPEG2"
 enabled libx265   && require_pkg_config libx265 x265 x265.h 
x265_api_get &&
  require_cpp_condition libx265 x265.h "X265_BUILD 
>= 89"
diff --git a/libavcodec/libx264.c b/libavcodec/libx264.c
index 182e20f622..9177c5d1c8 100644
--- a/libavcodec/libx264.c
+++ b/libavcodec/libx264.c
@@ -25,6 +25,7 @@
 #include "libavutil/eval.h"
 #include "libavutil/internal.h"
 #include "libavutil/opt.h"
+#include "libavutil/mastering_display_metadata.h"
 #include "libavutil/mem.h"
 #include "libavutil/pixdesc.h"
 #include "libavutil/stereo3d.h"
@@ -871,6 +872,81 @@ static int convert_pix_fmt(enum AVPixelFormat pix_fmt)
 return AVERROR(EINVAL);\
 }
 
+#if CONFIG_LIBX264_HDR10
+static void handle_mdcv(x264_param_t *params,
+const AVMasteringDisplayMetadata *mdcv)
+{
+if (!mdcv->has_primaries && !mdcv->has_luminance)
+return;
+
+params->mastering_display.b_mastering_display = 1;
+
+if (mdcv->has_primaries) {
+int *const points[][2] = {
+{
+¶ms->mastering_display.i_red_x,
+¶ms->mastering_display.i_red_y
+},
+{
+¶ms->mastering_display.i_green_x,
+¶ms->mastering_display.i_green_y
+},
+{
+¶ms->mastering_display.i_blue_x,
+¶ms->mastering_display.i_blue_y
+},
+};
+
+for (int i = 0; i < 3; i++) {
+const AVRational *src = mdcv->display_primaries[i];
+int *dst[2] = { points[i][0], points[i][1] };
+
+*dst[0] = av_rescale_q(1, src[0], (AVRational){ 1, 5 });
+*dst[1] = av_rescale_q(1, src[1], (AVRational){ 1, 5 });
+}
+
+params->mastering_display.i_white_x =
+av_rescale_q(1, mdcv->white_point[0], (AVRational){ 1, 5 });
+params->mastering_display.i_white_y =
+av_rescale_q(1, mdcv->white_point[1], (AVRational){ 1, 5 });
+}
+
+if (mdcv->has_luminance) {
+params->mastering_display.i_display_max =
+av_rescale_q(1, mdcv->max_luminance, (AVRational){ 1, 1 });
+params->mastering_display.i_display_min =
+av_rescale_q(1, mdcv->min_luminance, (AVRational){ 1, 1 });
+}
+}
+#endif // CONFIG_LIBX264_HDR10
+
+static void handle_side_data(AVCodecContext *avctx, x264_param_t *params)
+{
+#if CONFIG_LIBX264_HDR10
+const AVFrameSideDataSet set = avctx->frame_sd_set;
+const AVFrameSideData *cll_sd =
+av_frame_side_data_set_get_entry(
+set, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
+const AVFrameSideData *mdcv_sd =
+av_frame_side_data_set_get_entry(
+set, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
+
+if (cll_sd) {
+const AVContentLightMetadata *cll =
+(AVContentLightMetadata *)cll_sd->data;
+
+params->content_light_level.i_max_cll  = cll->MaxCLL;
+params->content_light_level.i_max_fall = cll->MaxFALL;
+
+params->content_light_level.b_cll = 1;
+}
+
+if (mdcv_sd) {
+handle_mdcv(params, (AVMasteringDisplayMetadata *)mdcv_sd->data);
+}
+#endif // CONFIG_LIBX264_HDR10
+}
+
 static av_cold int X264_init(AVCodecContext *avctx)
 {
 X264Context *x4 = avctx->priv_data;
@@ -1171,6 +1247,8 @@ FF_ENABLE_DEPRECATION_WARNINGS
 if (avctx->chroma_sample_location != AVCHROMA_LOC_UNSPECIFIED)
 x4->params.vui.i_chroma_loc = avctx->chroma_sample_location - 1;
 
+handle_side_data(av

[FFmpeg-devel] [PATCH v5 14/14] avcodec/libx265: add support for writing out CLL and MDCV

2023-11-26 Thread Jan Ekström
The newer of these two are the separate integers for content light
level, introduced in 3952bf3e98c76c31594529a3fe34e056d3e3e2ea ,
with X265_BUILD 75. As we already require X265_BUILD of at least
89, no further conditions are required.
---
 libavcodec/libx265.c | 87 
 tests/fate/enc_external.mak  |  5 +++
 tests/ref/fate/libx265-hdr10 | 16 +++
 3 files changed, 108 insertions(+)
 create mode 100644 tests/ref/fate/libx265-hdr10

diff --git a/libavcodec/libx265.c b/libavcodec/libx265.c
index 447e6da25f..96e50a6223 100644
--- a/libavcodec/libx265.c
+++ b/libavcodec/libx265.c
@@ -28,9 +28,11 @@
 #include 
 
 #include "libavutil/avassert.h"
+#include "libavutil/bprint.h"
 #include "libavutil/buffer.h"
 #include "libavutil/internal.h"
 #include "libavutil/common.h"
+#include "libavutil/mastering_display_metadata.h"
 #include "libavutil/opt.h"
 #include "libavutil/pixdesc.h"
 #include "avcodec.h"
@@ -179,6 +181,84 @@ static av_cold int libx265_param_parse_int(AVCodecContext 
*avctx,
 return 0;
 }
 
+static int handle_mdcv(const AVClass **avcl, const x265_api *api,
+   x265_param *params,
+   const AVMasteringDisplayMetadata *mdcv)
+{
+int ret = AVERROR_BUG;
+AVBPrint buf;
+av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC);
+
+// G(%hu,%hu)B(%hu,%hu)R(%hu,%hu)WP(%hu,%hu)L(%u,%u)
+av_bprintf(
+&buf,
+"G(%"PRId64",%"PRId64")B(%"PRId64",%"PRId64")R(%"PRId64",%"PRId64")"
+"WP(%"PRId64",%"PRId64")L(%"PRId64",%"PRId64")",
+av_rescale_q(1, mdcv->display_primaries[1][0], (AVRational){ 1, 5 
}),
+av_rescale_q(1, mdcv->display_primaries[1][1], (AVRational){ 1, 5 
}),
+av_rescale_q(1, mdcv->display_primaries[2][0], (AVRational){ 1, 5 
}),
+av_rescale_q(1, mdcv->display_primaries[2][1], (AVRational){ 1, 5 
}),
+av_rescale_q(1, mdcv->display_primaries[0][0], (AVRational){ 1, 5 
}),
+av_rescale_q(1, mdcv->display_primaries[0][1], (AVRational){ 1, 5 
}),
+av_rescale_q(1, mdcv->white_point[0], (AVRational){ 1, 5 }),
+av_rescale_q(1, mdcv->white_point[1], (AVRational){ 1, 5 }),
+av_rescale_q(1, mdcv->max_luminance,  (AVRational){ 1, 1 }),
+av_rescale_q(1, mdcv->min_luminance,  (AVRational){ 1, 1 }));
+
+if (!av_bprint_is_complete(&buf)) {
+av_log(avcl, AV_LOG_ERROR,
+  "MDCV string too long for its available space!\n");
+ret = AVERROR(ENOMEM);
+goto end;
+}
+
+if (api->param_parse(params, "master-display", buf.str) ==
+X265_PARAM_BAD_VALUE) {
+av_log(avcl, AV_LOG_ERROR,
+   "Invalid value \"%s\" for param \"master-display\".\n",
+   buf.str);
+ret = AVERROR(EINVAL);
+goto end;
+}
+
+ret = 0;
+
+end:
+av_bprint_finalize(&buf, NULL);
+
+return ret;
+}
+
+static int handle_side_data(AVCodecContext *avctx, const x265_api *api,
+x265_param *params)
+{
+const AVFrameSideDataSet set = avctx->frame_sd_set;
+const AVFrameSideData *cll_sd =
+av_frame_side_data_set_get_entry(
+set, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL);
+const AVFrameSideData *mdcv_sd =
+av_frame_side_data_set_get_entry(
+set, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA);
+
+if (cll_sd) {
+const AVContentLightMetadata *cll =
+(AVContentLightMetadata *)cll_sd->data;
+
+params->maxCLL  = cll->MaxCLL;
+params->maxFALL = cll->MaxFALL;
+}
+
+if (mdcv_sd) {
+int ret = handle_mdcv(
+&avctx->av_class, api, params,
+(AVMasteringDisplayMetadata *)mdcv_sd->data);
+if (ret < 0)
+return ret;
+}
+
+return 0;
+}
+
 static av_cold int libx265_encode_init(AVCodecContext *avctx)
 {
 libx265Context *ctx = avctx->priv_data;
@@ -339,6 +419,13 @@ FF_ENABLE_DEPRECATION_WARNINGS
 return AVERROR_BUG;
 }
 
+ret = handle_side_data(avctx, ctx->api, ctx->params);
+if (ret < 0) {
+av_log(avctx, AV_LOG_ERROR, "Failed handling side data! (%s)\n",
+   av_err2str(ret));
+return ret;
+}
+
 if (ctx->crf >= 0) {
 char crf[6];
 
diff --git a/tests/fate/enc_external.mak b/tests/fate/enc_external.mak
index 4095a4b51a..30021efbcd 100644
--- a/tests/fate/enc_external.mak
+++ b/tests/fate/enc_external.mak
@@ -12,5 +12,10 @@ FATE_ENC_EXTERNAL-$(call ENCDEC, LIBX264 HEVC, MOV, 
LIBX264_HDR10 HEVC_DEMUXER H
 fate-libx264-hdr10: CMD = enc_external 
$(TARGET_SAMPLES)/hevc/hdr10_plus_h265_sample.hevc \
 mp4 "-c:v libx264" "-show_frames -show_entries frame=side_data_list -of 
flat"
 
+# test for x265 MDCV and CLL passthrough during encoding
+FATE_ENC_EXTERNAL-$(call ENCDEC, LIBX265 HEVC, MOV, HEVC_DEMUXER) += 
fate-libx265-hdr10
+fate-libx265-hdr10: CMD = enc_external 
$(TARGET_SAMPLES)/hevc/hdr10_plus_h26

[FFmpeg-devel] [PATCH] mailmap: add entry for myself

2023-12-07 Thread Jan Ekström
---
 .mailmap | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.mailmap b/.mailmap
index 7546cf0caf..307798b1ee 100644
--- a/.mailmap
+++ b/.mailmap
@@ -1,4 +1,5 @@
  
+ 
  
  
  
-- 
2.43.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 v2 0/3] Initial support for fragmented TTML muxing

2023-12-08 Thread Jan Ekström
On Fri, Sep 15, 2023 at 4:17 PM Jan Ekström  wrote:
>
> Changes compared to v1:
>
> * General rebase.
> * A FATE test was added, together with the extension of the "transcode"
>   test function to allow for dumping of packets' contents.
> * Simplified mov_write_ttml_document_from_queue's loop by getting
>   rid of `stop_at_current_packet`.
>
> This enables pushing TTML together with another track (usually video)
> as part of CMAF Ingest, as defined by the DASH-IF Live Media Ingest
> Protocol.
>
> Currently does not function well with just the subtitle track unless
> the API user explicitly requests fragmentation with a nullptr packet,
> as the generic fragmentation decision logic is based on tracks which
> do not require squashing.
>
> Currently does support overlapping subtitles, but the implementation
> utilizes another packet queue for it, which is probably not optimal.
> Recommendations on how to improve things are welcome.
>
> Jan
>

Ping.

As this is a piece of seemingly working functionality, I'd like to
understand whether people think subtitle-only fragmented MP4 documents
with more than one fragment is something that is required for this to
get merged.

Also I have changed an existing test function, so I'd like a note
whether people think this is an OK change.

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


Re: [FFmpeg-devel] [PATCH v5 00/14] encoder AVCodecContext configuration side data

2023-12-08 Thread Jan Ekström
On Sun, Nov 26, 2023 at 9:58 PM Jan Ekström  wrote:
>
> Differences to v3:
> 1. rebased on top of current master
> 2. moved the addition of multiple side data entries from a generic
>av_frame_side_data_set_extend to avcodec as per request from James.
> 4. adopted various things noted by reviews
>
> Comparison URL (mostly configure and wrappers, avutil/frame.c):
> https://github.com/jeeb/ffmpeg/compare/avcodec_cll_mdcv_side_data_v4..avcodec_cll_mdcv_side_data_v5
>
> This patch set I've now been working for a while since I felt like it was 
> weird
> we couldn't pass through information such as static HDR metadata to encoders
> from decoded input. This initial version adds the necessary framework, as well
> as adds static HDR metadata support for libsvtav1, libx264 as well as libx265
> wrappers.
>
> An alternative to this would be to make encoders only properly initialize when
> they receive the first AVFrame, but that seems to be a bigger, nastier change
> than introducing an AVFrameSideDataSet in avctx as everything seems to
> presume that extradata etc are available after opening the encoder.
>
> Note: Any opinions on whether FFCodec or AVCodec should have
>   handled_side_data list, so that if format specific generic logic is
>   added, it could be checked whether the codec itself handles this side
>   data? This could also be utilized to list handled side data from f.ex.
>   `ffmpeg -h encoder=libsvtav1`.
>
> Jan

Ping.

I'd like to understand whether:

* people are fine with the struct which lets you pass things as a
single argument, or they would like to move all the helper functions
to counter and pointer.
* should the avcodec helper take in the struct/{counter,pointer}, or
should it instead take in a const AVFrame pointer?

as I'd like to start pulling this in, and move towards working on
implemented side data listing in AVCodecs, as well as adding generic
implementations for specific codecs (such as H.264/HEVC/AV1 having CBS
create side data generically, without the need of specific AVCodecs
implementing things),

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


Re: [FFmpeg-devel] [PATCH v2 3/3] avformat/movenc: add support for fragmented TTML muxing

2023-12-08 Thread Jan Ekström
On Fri, Dec 8, 2023 at 5:37 PM Dennis Mungai  wrote:
>
> On Fri, 8 Dec 2023 at 15:14, Andreas Rheinhardt <
> andreas.rheinha...@outlook.com> wrote:
>
> > Jan Ekström:
> > > From: Jan Ekström 
> > >
> > > Attempts to base the fragmentation timing on other streams
> > > as most receivers expect media fragments to be more or less
> > > aligned.
> > >
> > > Currently does not support fragmentation on subtitle track
> > > only, as the subtitle packet queue timings would have to be
> > > checked in addition to the current fragmentation timing logic.
> > >
> > > Signed-off-by: Jan Ekström 
> > > ---
> > >  libavformat/movenc.c|9 -
> > >  libavformat/movenc_ttml.c   |  157 ++-
> > >  tests/fate/mov.mak  |   21 +
> > >  tests/ref/fate/mov-mp4-fragmented-ttml-dfxp | 1197 +++
> > >  tests/ref/fate/mov-mp4-fragmented-ttml-stpp | 1197 +++
> >
> > Am I the only one who thinks that this is a bit excessive?
> >
> > >  5 files changed, 2568 insertions(+), 13 deletions(-)
> > >  create mode 100644 tests/ref/fate/mov-mp4-fragmented-ttml-dfxp
> > >  create mode 100644 tests/ref/fate/mov-mp4-fragmented-ttml-stpp
> > >
> > > diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
> > > index 6cb493ceab..5c44299196 100644
> > > --- a/tests/fate/mov.mak
> > > +++ b/tests/fate/mov.mak
> > > @@ -143,6 +143,27 @@ FATE_MOV_FFMPEG_FFPROBE-$(call TRANSCODE, TTML
> > SUBRIP, MP4 MOV, SRT_DEMUXER TTML
> > >  fate-mov-mp4-ttml-stpp: CMD = transcode srt
> > $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 "-map 0:s -c:s ttml
> > -time_base:s 1:1000" "-map 0 -c copy" "-of json -show_entries
> > packet:stream=index,codec_type,codec_tag_string,codec_tag,codec_name,time_base,start_time,duration_ts,duration,nb_frames,nb_read_packets:stream_tags"
> > >  fate-mov-mp4-ttml-dfxp: CMD = transcode srt
> > $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 "-map 0:s -c:s ttml
> > -time_base:s 1:1000 -tag:s dfxp -strict unofficial" "-map 0 -c copy" "-of
> > json -show_entries
> > packet:stream=index,codec_type,codec_tag_string,codec_tag,codec_name,time_base,start_time,duration_ts,duration,nb_frames,nb_read_packets:stream_tags"
> > >
> > > +FATE_MOV_FFMPEG_FFPROBE-$(call TRANSCODE, TTML SUBRIP, MP4 MOV,
> > LAVFI_INDEV SMPTEHDBARS_FILTER SRT_DEMUXER MPEG2VIDEO_ENCODER TTML_MUXER
> > RAWVIDEO_MUXER) += fate-mov-mp4-fragmented-ttml-stpp
> > > +fate-mov-mp4-fragmented-ttml-stpp: CMD = transcode srt
> > $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 \
> > > +  "-map 1:v -map 0:s \
> > > +   -c:v mpeg2video -b:v 2M -g 48 -sc_threshold 10 \
> > > +   -c:s ttml -time_base:s 1:1000 \
> > > +   -movflags +cmaf" \
> > > +  "-map 0:s -c copy" \
> > > +  "-select_streams s -of csv -show_packets -show_data_hash crc32" \
> > > +  "-f lavfi -i
> > smptehdbars=duration=70:size=320x180:rate=24000/1001,format=yuv420p" \
> > > +  "" "" "rawvideo"
> >
> > Would it speed the test up if you used smaller dimensions or a smaller
> > bitrate?
> > Anyway, you probably want the "data" output format instead of rawvideo.
> >
> > > +
> > > +FATE_MOV_FFMPEG_FFPROBE-$(call TRANSCODE, TTML SUBRIP, ISMV MOV,
> > LAVFI_INDEV SMPTEHDBARS_FILTER SRT_DEMUXER MPEG2VIDEO_ENCODER TTML_MUXER
> > RAWVIDEO_MUXER) += fate-mov-mp4-fragmented-ttml-dfxp
> > > +fate-mov-mp4-fragmented-ttml-dfxp: CMD = transcode srt
> > $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt ismv \
> > > +  "-map 1:v -map 0:s \
> > > +   -c:v mpeg2video -b:v 2M -g 48 -sc_threshold 10 \
> > > +   -c:s ttml -tag:s dfxp -time_base:s 1:1000" \
> > > +  "-map 0:s -c copy" \
> > > +  "-select_streams s -of csv -show_packets -show_data_hash crc32" \
> > > +  "-f lavfi -i
> > smptehdbars=duration=70:size=320x180:rate=24000/1001,format=yuv420p" \
> > > +  "" "" "rawvideo"
> > > +
> > >  # FIXME: Uncomment these two tests once the test files are uploaded to
> > the fate
> > >  # server.
> > >  # avif demuxing - still image with 1 item.
> >
>
> Hello Jan,
>
> Taking this note into account, and I quote:
>
>  " Currently does not 

Re: [FFmpeg-devel] [PATCH v2 3/3] avformat/movenc: add support for fragmented TTML muxing

2023-12-08 Thread Jan Ekström
On Fri, Dec 8, 2023 at 7:05 PM Jan Ekström  wrote:
>
> On Fri, Dec 8, 2023 at 5:37 PM Dennis Mungai  wrote:
> >
> > On Fri, 8 Dec 2023 at 15:14, Andreas Rheinhardt <
> > andreas.rheinha...@outlook.com> wrote:
> >
> > > Jan Ekström:
> > > > From: Jan Ekström 
> > > >
> > > > Attempts to base the fragmentation timing on other streams
> > > > as most receivers expect media fragments to be more or less
> > > > aligned.
> > > >
> > > > Currently does not support fragmentation on subtitle track
> > > > only, as the subtitle packet queue timings would have to be
> > > > checked in addition to the current fragmentation timing logic.
> > > >
> > > > Signed-off-by: Jan Ekström 
> > > > ---
> > > >  libavformat/movenc.c|9 -
> > > >  libavformat/movenc_ttml.c   |  157 ++-
> > > >  tests/fate/mov.mak  |   21 +
> > > >  tests/ref/fate/mov-mp4-fragmented-ttml-dfxp | 1197 +++
> > > >  tests/ref/fate/mov-mp4-fragmented-ttml-stpp | 1197 +++
> > >
> > > Am I the only one who thinks that this is a bit excessive?
> > >
> > > >  5 files changed, 2568 insertions(+), 13 deletions(-)
> > > >  create mode 100644 tests/ref/fate/mov-mp4-fragmented-ttml-dfxp
> > > >  create mode 100644 tests/ref/fate/mov-mp4-fragmented-ttml-stpp
> > > >
> > > > diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
> > > > index 6cb493ceab..5c44299196 100644
> > > > --- a/tests/fate/mov.mak
> > > > +++ b/tests/fate/mov.mak
> > > > @@ -143,6 +143,27 @@ FATE_MOV_FFMPEG_FFPROBE-$(call TRANSCODE, TTML
> > > SUBRIP, MP4 MOV, SRT_DEMUXER TTML
> > > >  fate-mov-mp4-ttml-stpp: CMD = transcode srt
> > > $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 "-map 0:s -c:s ttml
> > > -time_base:s 1:1000" "-map 0 -c copy" "-of json -show_entries
> > > packet:stream=index,codec_type,codec_tag_string,codec_tag,codec_name,time_base,start_time,duration_ts,duration,nb_frames,nb_read_packets:stream_tags"
> > > >  fate-mov-mp4-ttml-dfxp: CMD = transcode srt
> > > $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 "-map 0:s -c:s ttml
> > > -time_base:s 1:1000 -tag:s dfxp -strict unofficial" "-map 0 -c copy" "-of
> > > json -show_entries
> > > packet:stream=index,codec_type,codec_tag_string,codec_tag,codec_name,time_base,start_time,duration_ts,duration,nb_frames,nb_read_packets:stream_tags"
> > > >
> > > > +FATE_MOV_FFMPEG_FFPROBE-$(call TRANSCODE, TTML SUBRIP, MP4 MOV,
> > > LAVFI_INDEV SMPTEHDBARS_FILTER SRT_DEMUXER MPEG2VIDEO_ENCODER TTML_MUXER
> > > RAWVIDEO_MUXER) += fate-mov-mp4-fragmented-ttml-stpp
> > > > +fate-mov-mp4-fragmented-ttml-stpp: CMD = transcode srt
> > > $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 \
> > > > +  "-map 1:v -map 0:s \
> > > > +   -c:v mpeg2video -b:v 2M -g 48 -sc_threshold 10 \
> > > > +   -c:s ttml -time_base:s 1:1000 \
> > > > +   -movflags +cmaf" \
> > > > +  "-map 0:s -c copy" \
> > > > +  "-select_streams s -of csv -show_packets -show_data_hash crc32" \
> > > > +  "-f lavfi -i
> > > smptehdbars=duration=70:size=320x180:rate=24000/1001,format=yuv420p" \
> > > > +  "" "" "rawvideo"
> > >
> > > Would it speed the test up if you used smaller dimensions or a smaller
> > > bitrate?
> > > Anyway, you probably want the "data" output format instead of rawvideo.
> > >
> > > > +
> > > > +FATE_MOV_FFMPEG_FFPROBE-$(call TRANSCODE, TTML SUBRIP, ISMV MOV,
> > > LAVFI_INDEV SMPTEHDBARS_FILTER SRT_DEMUXER MPEG2VIDEO_ENCODER TTML_MUXER
> > > RAWVIDEO_MUXER) += fate-mov-mp4-fragmented-ttml-dfxp
> > > > +fate-mov-mp4-fragmented-ttml-dfxp: CMD = transcode srt
> > > $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt ismv \
> > > > +  "-map 1:v -map 0:s \
> > > > +   -c:v mpeg2video -b:v 2M -g 48 -sc_threshold 10 \
> > > > +   -c:s ttml -tag:s dfxp -time_base:s 1:1000" \
> > > > +  "-map 0:s -c copy" \
> > > > +  "-select_streams s -of csv -show_packets -show_data_hash crc32" \
> > > > +  "-f lavfi -i
> > > smptehdbars=duration=70

Re: [FFmpeg-devel] [PATCH v2 3/3] avformat/movenc: add support for fragmented TTML muxing

2023-12-08 Thread Jan Ekström
On Fri, Dec 8, 2023 at 7:09 PM Jan Ekström  wrote:
>
> On Fri, Dec 8, 2023 at 7:05 PM Jan Ekström  wrote:
> >
> > On Fri, Dec 8, 2023 at 5:37 PM Dennis Mungai  wrote:
> > >
> > > On Fri, 8 Dec 2023 at 15:14, Andreas Rheinhardt <
> > > andreas.rheinha...@outlook.com> wrote:
> > >
> > > > Jan Ekström:
> > > > > From: Jan Ekström 
> > > > >
> > > > > Attempts to base the fragmentation timing on other streams
> > > > > as most receivers expect media fragments to be more or less
> > > > > aligned.
> > > > >
> > > > > Currently does not support fragmentation on subtitle track
> > > > > only, as the subtitle packet queue timings would have to be
> > > > > checked in addition to the current fragmentation timing logic.
> > > > >
> > > > > Signed-off-by: Jan Ekström 
> > > > > ---
> > > > >  libavformat/movenc.c|9 -
> > > > >  libavformat/movenc_ttml.c   |  157 ++-
> > > > >  tests/fate/mov.mak  |   21 +
> > > > >  tests/ref/fate/mov-mp4-fragmented-ttml-dfxp | 1197 
> > > > > +++
> > > > >  tests/ref/fate/mov-mp4-fragmented-ttml-stpp | 1197 
> > > > > +++
> > > >
> > > > Am I the only one who thinks that this is a bit excessive?
> > > >
> > > > >  5 files changed, 2568 insertions(+), 13 deletions(-)
> > > > >  create mode 100644 tests/ref/fate/mov-mp4-fragmented-ttml-dfxp
> > > > >  create mode 100644 tests/ref/fate/mov-mp4-fragmented-ttml-stpp
> > > > >
> > > > > diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
> > > > > index 6cb493ceab..5c44299196 100644
> > > > > --- a/tests/fate/mov.mak
> > > > > +++ b/tests/fate/mov.mak
> > > > > @@ -143,6 +143,27 @@ FATE_MOV_FFMPEG_FFPROBE-$(call TRANSCODE, TTML
> > > > SUBRIP, MP4 MOV, SRT_DEMUXER TTML
> > > > >  fate-mov-mp4-ttml-stpp: CMD = transcode srt
> > > > $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 "-map 0:s -c:s 
> > > > ttml
> > > > -time_base:s 1:1000" "-map 0 -c copy" "-of json -show_entries
> > > > packet:stream=index,codec_type,codec_tag_string,codec_tag,codec_name,time_base,start_time,duration_ts,duration,nb_frames,nb_read_packets:stream_tags"
> > > > >  fate-mov-mp4-ttml-dfxp: CMD = transcode srt
> > > > $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 "-map 0:s -c:s 
> > > > ttml
> > > > -time_base:s 1:1000 -tag:s dfxp -strict unofficial" "-map 0 -c copy" 
> > > > "-of
> > > > json -show_entries
> > > > packet:stream=index,codec_type,codec_tag_string,codec_tag,codec_name,time_base,start_time,duration_ts,duration,nb_frames,nb_read_packets:stream_tags"
> > > > >
> > > > > +FATE_MOV_FFMPEG_FFPROBE-$(call TRANSCODE, TTML SUBRIP, MP4 MOV,
> > > > LAVFI_INDEV SMPTEHDBARS_FILTER SRT_DEMUXER MPEG2VIDEO_ENCODER TTML_MUXER
> > > > RAWVIDEO_MUXER) += fate-mov-mp4-fragmented-ttml-stpp
> > > > > +fate-mov-mp4-fragmented-ttml-stpp: CMD = transcode srt
> > > > $(TARGET_SAMPLES)/sub/SubRip_capability_tester.srt mp4 \
> > > > > +  "-map 1:v -map 0:s \
> > > > > +   -c:v mpeg2video -b:v 2M -g 48 -sc_threshold 10 \
> > > > > +   -c:s ttml -time_base:s 1:1000 \
> > > > > +   -movflags +cmaf" \
> > > > > +  "-map 0:s -c copy" \
> > > > > +  "-select_streams s -of csv -show_packets -show_data_hash crc32" \
> > > > > +  "-f lavfi -i
> > > > smptehdbars=duration=70:size=320x180:rate=24000/1001,format=yuv420p" \
> > > > > +  "" "" "rawvideo"
> > > >
> > > > Would it speed the test up if you used smaller dimensions or a smaller
> > > > bitrate?
> > > > Anyway, you probably want the "data" output format instead of rawvideo.
> > > >
> > > > > +
> > > > > +FATE_MOV_FFMPEG_FFPROBE-$(call TRANSCODE, TTML SUBRIP, ISMV MOV,
> > > > LAVFI_INDEV SMPTEHDBARS_FILTER SRT_DEMUXER MPEG2VIDEO_ENCODER TTML_MUXER
> > > > RAWVIDEO_MUXER) += fate-mov-mp4-fragmented-ttml-dfxp
> > > > > +fate-mov-mp4-fragmented-ttml-dfxp: CMD

[FFmpeg-devel] [PATCH v3 0/3] Initial support for fragmented TTML muxing

2023-12-11 Thread Jan Ekström
Changes compared to v2:

* General rebase.
* Limited the test to the first overlapping lines that
  finish at around 24.5 seconds. This brings us down to 430 lines
  per test (of which there are two).
* Switched from the rawvideo to the data muxer.

This enables pushing TTML together with another track (usually video)
as part of CMAF Ingest, as defined by the DASH-IF Live Media Ingest
Protocol.

Currently does not function well with just the subtitle track unless
the API user explicitly requests fragmentation with a nullptr packet,
as the generic fragmentation decision logic is based on tracks which
do not require squashing.

Currently does support overlapping subtitles, but the implementation
utilizes another packet queue for it, which is probably not optimal.
Recommendations on how to improve things are welcome.

Jan

Jan Ekström (3):
  tests/fate-run: add support for specifying the final encode muxer in
`transcode`
  avcodec/avpacket: add functionality to prepend to AVPacketLists
  avformat/movenc: add support for fragmented TTML muxing

 libavcodec/avpacket.c   |  20 +-
 libavcodec/packet_internal.h|   2 +
 libavformat/movenc.c|   9 -
 libavformat/movenc_ttml.c   | 157 ++-
 tests/fate-run.sh   |   4 +-
 tests/fate/mov.mak  |  21 +
 tests/ref/fate/mov-mp4-fragmented-ttml-dfxp | 430 
 tests/ref/fate/mov-mp4-fragmented-ttml-stpp | 430 
 8 files changed, 1054 insertions(+), 19 deletions(-)
 create mode 100644 tests/ref/fate/mov-mp4-fragmented-ttml-dfxp
 create mode 100644 tests/ref/fate/mov-mp4-fragmented-ttml-stpp

-- 
2.43.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 v3 1/3] tests/fate-run: add support for specifying the final encode muxer in `transcode`

2023-12-11 Thread Jan Ekström
From: Jan Ekström 

This allows for direct dumping of the packets' contents (useful for
text based formats), while getting the timestamps/sizes etc from
ffprobe.

If used via TRANSCODE, the actually utilized muxer should be added
within the last argument as an additional dependency, as that is not
done automatically.

Signed-off-by: Jan Ekström 
---
 tests/fate-run.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/fate-run.sh b/tests/fate-run.sh
index 8efb1586b8..73c4d16f21 100755
--- a/tests/fate-run.sh
+++ b/tests/fate-run.sh
@@ -257,7 +257,9 @@ transcode(){
 additional_input=$7
 final_decode=$8
 enc_opt_in=$9
+final_encode_muxer="${10}"
 test -z "$additional_input" || additional_input="$DEC_OPTS 
$additional_input"
+test -z "$final_encode_muxer" && final_encode_muxer="framecrc"
 encfile="${outdir}/${test}.${enc_fmt}"
 test $keep -ge 1 || cleanfiles="$cleanfiles $encfile"
 tsrcfile=$(target_path $srcfile)
@@ -267,7 +269,7 @@ transcode(){
 do_md5sum $encfile
 echo $(wc -c $encfile)
 ffmpeg $DEC_OPTS $final_decode -i $tencfile $ENC_OPTS $FLAGS $final_encode 
\
--f framecrc - || return
+-f $final_encode_muxer - || return
 test -z $ffprobe_opts || \
 run ffprobe${PROGSUF}${EXECSUF} -bitexact $ffprobe_opts $tencfile || 
return
 }
-- 
2.43.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 v3 2/3] avcodec/avpacket: add functionality to prepend to AVPacketLists

2023-12-11 Thread Jan Ekström
From: Jan Ekström 

Signed-off-by: Jan Ekström 
---
 libavcodec/avpacket.c| 20 +++-
 libavcodec/packet_internal.h |  2 ++
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/libavcodec/avpacket.c b/libavcodec/avpacket.c
index e29725c2d2..e223ab63ef 100644
--- a/libavcodec/avpacket.c
+++ b/libavcodec/avpacket.c
@@ -540,6 +540,7 @@ int avpriv_packet_list_put(PacketList *packet_buffer,
int flags)
 {
 PacketListEntry *pktl = av_malloc(sizeof(*pktl));
+unsigned int update_end_point = 1;
 int ret;
 
 if (!pktl)
@@ -563,13 +564,22 @@ int avpriv_packet_list_put(PacketList *packet_buffer,
 
 pktl->next = NULL;
 
-if (packet_buffer->head)
-packet_buffer->tail->next = pktl;
-else
+if (packet_buffer->head) {
+if (flags & FF_PACKETLIST_FLAG_PREPEND) {
+pktl->next = packet_buffer->head;
+packet_buffer->head = pktl;
+update_end_point = 0;
+} else {
+packet_buffer->tail->next = pktl;
+}
+} else
 packet_buffer->head = pktl;
 
-/* Add the packet in the buffered packet list. */
-packet_buffer->tail = pktl;
+if (update_end_point) {
+/* Add the packet in the buffered packet list. */
+packet_buffer->tail = pktl;
+}
+
 return 0;
 }
 
diff --git a/libavcodec/packet_internal.h b/libavcodec/packet_internal.h
index 52fa6d9be9..9c0f4fead5 100644
--- a/libavcodec/packet_internal.h
+++ b/libavcodec/packet_internal.h
@@ -34,6 +34,8 @@ typedef struct PacketList {
 PacketListEntry *head, *tail;
 } PacketList;
 
+#define FF_PACKETLIST_FLAG_PREPEND (1 << 0) /**< Prepend created AVPacketList 
instead of appending */
+
 /**
  * Append an AVPacket to the list.
  *
-- 
2.43.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 v3 3/3] avformat/movenc: add support for fragmented TTML muxing

2023-12-11 Thread Jan Ekström
From: Jan Ekström 

Attempts to base the fragmentation timing on other streams
as most receivers expect media fragments to be more or less
aligned.

Currently does not support fragmentation on subtitle track
only, as the subtitle packet queue timings would have to be
checked in addition to the current fragmentation timing logic.

Signed-off-by: Jan Ekström 
---
 libavformat/movenc.c|   9 -
 libavformat/movenc_ttml.c   | 157 ++-
 tests/fate/mov.mak  |  21 +
 tests/ref/fate/mov-mp4-fragmented-ttml-dfxp | 430 
 tests/ref/fate/mov-mp4-fragmented-ttml-stpp | 430 
 5 files changed, 1034 insertions(+), 13 deletions(-)
 create mode 100644 tests/ref/fate/mov-mp4-fragmented-ttml-dfxp
 create mode 100644 tests/ref/fate/mov-mp4-fragmented-ttml-stpp

diff --git a/libavformat/movenc.c b/libavformat/movenc.c
index e39f1ac987..0b0006fd93 100644
--- a/libavformat/movenc.c
+++ b/libavformat/movenc.c
@@ -7324,15 +7324,6 @@ static int mov_init(AVFormatContext *s)
 track->squash_fragment_samples_to_one =
 ff_is_ttml_stream_paragraph_based(track->par);
 
-if (mov->flags & FF_MOV_FLAG_FRAGMENT &&
-track->squash_fragment_samples_to_one) {
-av_log(s, AV_LOG_ERROR,
-   "Fragmentation is not currently supported for "
-   "TTML in MP4/ISMV (track synchronization between "
-   "subtitles and other media is not yet 
implemented)!\n");
-return AVERROR_PATCHWELCOME;
-}
-
 if (track->mode != MODE_ISM &&
 track->par->codec_tag == MOV_ISMV_TTML_TAG &&
 s->strict_std_compliance > FF_COMPLIANCE_UNOFFICIAL) {
diff --git a/libavformat/movenc_ttml.c b/libavformat/movenc_ttml.c
index 6deae49657..2aefb0b6c3 100644
--- a/libavformat/movenc_ttml.c
+++ b/libavformat/movenc_ttml.c
@@ -54,6 +54,50 @@ static int mov_init_ttml_writer(MOVTrack *track, 
AVFormatContext **out_ctx)
 return 0;
 }
 
+static void mov_calculate_start_and_end_of_other_tracks(
+AVFormatContext *s, MOVTrack *track, int64_t *start_pts, int64_t *end_pts)
+{
+MOVMuxContext *mov = s->priv_data;
+
+// Initialize at the end of the previous document/fragment, which is NOPTS
+// until the first fragment is created.
+int64_t max_track_end_dts = *start_pts = track->end_pts;
+
+for (unsigned int i = 0; i < s->nb_streams; i++) {
+MOVTrack *other_track = &mov->tracks[i];
+
+// Skip our own track, any other track that needs squashing,
+// or any track which still has its start_dts at NOPTS or
+// any track that did not yet get any packets.
+if (track == other_track ||
+other_track->squash_fragment_samples_to_one ||
+other_track->start_dts == AV_NOPTS_VALUE ||
+!other_track->entry) {
+continue;
+}
+
+{
+int64_t picked_start = 
av_rescale_q_rnd(other_track->cluster[0].dts + other_track->cluster[0].cts,
+other_track->st->time_base,
+track->st->time_base,
+AV_ROUND_NEAR_INF | 
AV_ROUND_PASS_MINMAX);
+int64_t picked_end   = av_rescale_q_rnd(other_track->end_pts,
+other_track->st->time_base,
+track->st->time_base,
+AV_ROUND_NEAR_INF | 
AV_ROUND_PASS_MINMAX);
+
+if (*start_pts == AV_NOPTS_VALUE)
+*start_pts = picked_start;
+else if (picked_start >= track->end_pts)
+*start_pts = FFMIN(*start_pts, picked_start);
+
+max_track_end_dts = FFMAX(max_track_end_dts, picked_end);
+}
+}
+
+*end_pts = max_track_end_dts;
+}
+
 static int mov_write_ttml_document_from_queue(AVFormatContext *s,
   AVFormatContext *ttml_ctx,
   MOVTrack *track,
@@ -65,13 +109,85 @@ static int 
mov_write_ttml_document_from_queue(AVFormatContext *s,
 int64_t start_ts = track->start_dts == AV_NOPTS_VALUE ?
0 : (track->start_dts + track->track_duration);
 int64_t end_ts   = start_ts;
+unsigned int time_limited = 0;
+PacketList back_to_queue_list = { 0 };
+
+if (*out_start_ts != AV_NOPTS_VALUE) {
+// we have non-nopts values here, thus we have been given a time range
+time_limited = 1;
+start_ts = *out_start_ts;
+

Re: [FFmpeg-devel] avcodec/videotoolbox: add support for full range and 10bit pixel formats

2019-04-09 Thread Jan Ekström
Hi,

On Wed, Mar 27, 2019 at 11:34 PM der richter  wrote:
>
> apparently the first patch didn't make it through
>

Sorry for this taking a while.

This has been generally tested to build and work with XCode 10.1 (I
think this is the newest version still available for 10.13.x) and
generally seems to follow the style of what's been done before.

Only nit regarding the pix_fmt descriptor: since other parts of the
module seem to utilize the !pointer style of checking and early exit,
I think you should switch

> if (descriptor != NULL) {
> int depth = descriptor->comp[0].depth;
> if (depth > 8) {
> return AV_PIX_FMT_P010;
> }
> }
>
> return AV_PIX_FMT_NV12; // same as av_videotoolbox_alloc_context()

to:

if (!descriptor)
return AV_PIX_FMT_NV12; // same as av_videotoolbox_alloc_context()

int depth = descriptor->comp[0].depth;
if (depth > 8) {
return AV_PIX_FMT_P010;
}

or so.

Best regards,
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".

Re: [FFmpeg-devel] avcodec/videotoolbox: add support for full range and 10bit pixel formats

2019-04-09 Thread Jan Ekström
On Wed, Mar 27, 2019 at 11:32 PM der richter  wrote:
>
> the second patch
>

Generally looks like a nice addition and apparently VideoToolbox seems
to be properly supporting this sort of content (at least according to
the results of https://github.com/mpv-player/mpv/issues/6546 ).

The first part that I'm not really sure of is if:

>
> --- a/libavutil/hwcontext_videotoolbox.h
> +++ b/libavutil/hwcontext_videotoolbox.h
> @@ -49,6 +49,6 @@ enum AVPixelFormat 
> av_map_videotoolbox_format_to_pixfmt(uint32_t cv_fmt);
>   * Convert an AVPixelFormat to a VideoToolbox (actually CoreVideo) format.
>   * Returns 0 if no known equivalent was found.
>   */
> -uint32_t av_map_videotoolbox_format_from_pixfmt(enum AVPixelFormat pix_fmt);
> +uint32_t av_map_videotoolbox_format_from_pixfmt(enum AVPixelFormat pix_fmt, 
> bool full_range);
>
>  #endif /* AVUTIL_HWCONTEXT_VIDEOTOOLBOX_H */

constitutes an API change and thus requires some sort of bump?

Best regards,
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".

Re: [FFmpeg-devel] [PATCH v3 1/2] avcodec/videotoolbox: add support for 10bit pixel format

2019-04-15 Thread Jan Ekström
On Sat, Apr 13, 2019 at 5:36 PM der richter  wrote:
>
> From: fumoboy007 
>
> this patch was originally posted on issue #7704 and was slightly
> adjusted to check for the availability of the pixel format.
> ---

Functionally tested on a macOS 10.13.x + XCode 10.1, as well as
forcibly made the kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange
check fail, and it still built fine. 10bit HEVC content is now
properly getting decoded onto P010 surfaces through VT instead of
getting dithered to NV12.

In other words, from my side LGTM.

Best regards,
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".

Re: [FFmpeg-devel] [PATCH v3 1/2] avcodec/videotoolbox: add support for 10bit pixel format

2019-04-15 Thread Jan Ekström
On Tue, Apr 16, 2019 at 2:55 AM Jan Ekström  wrote:
>
> On Sat, Apr 13, 2019 at 5:36 PM der richter  wrote:
> >
> > From: fumoboy007 
> >
> > this patch was originally posted on issue #7704 and was slightly
> > adjusted to check for the availability of the pixel format.
> > ---
>
> Functionally tested on a macOS 10.13.x + XCode 10.1, as well as
> forcibly made the kCVPixelFormatType_420YpCbCr10BiPlanarVideoRange
> check fail, and it still built fine. 10bit HEVC content is now
> properly getting decoded onto P010 surfaces through VT instead of
> getting dithered to NV12.
>
> In other words, from my side LGTM.
>
> Best regards,
> Jan

Additionally tested with ffmpeg.c and P010 images seem to be flowing
with hwaccel videotoolbox.

Applied as 036b4b0f85933f49a7094b5b568a93f68c9cd544 .

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

Re: [FFmpeg-devel] [DECISION] Project policy on closed source components

2019-05-08 Thread Jan Ekström
On Sun, Apr 28, 2019 at 11:02 PM Marton Balint  wrote:
>
> Hi All,
>
> There has been discussion on the mailing list several times about the
> inclusion of support for closed source components (codecs, formats,
> filters, etc) in the main ffmpeg codebase.
>
> Also the removal of libNDI happened without general consensus, so a vote
> is necessary to justify the removal.
>
> So here is a call to the voting committee [1] to decide on the following
> two questions:
>
> 1) Should libNDI support be removed from the ffmpeg codebase?
>

Yes.

To give a reasoning for this, I have taken a quick look at the history
of enable-nonfree (first appearing in
3fe142e2555ec8b527f2ff4cc517c015a114e91a in January of 2008), and it
seems like its reasoning was to enable linking of (open source)
components that were already in the code base that were then found out
to not have technical limitations in their build which would follow
their incompatibility with LGPL, GPL or both (starting with the 3GPP
AMR-NB decoder wrapper added in
891f64b33972bb35f64d0b7ae0928004ff278f5b in May of 2003).

Later on, it would be utilized when similar incompatibilities were
found, of which at this point in time the Google published Fraunhofer
AAC decoder/encoder suite is probably the best known example of.
Before that there was the case where people happened to look into what
libfaac actually contained, and it was noticed that the library
actually contained the code from the reference implementation, making
it incompatible with its own license.

Following that quick check of things, I looked at the licenses of what
FFmpeg can be redistributably built under: LGPL versions 2.1 and 3, as
well as GPLv2 and 3. Thus, effectively the most limiting license of
these would be one of the versions of the GPL. Thus, in my opinion as
much of the code in FFmpeg should be compatible with LGPLv2.1+ and
GPLv2+. And thus we gain an understanding of what sort of closed
source software we can utilize within FFmpeg without limiting the
option in binary licenses for the user (basically, the poorly worded
things regarding things that come with the system - which is why
generally having schannel/dxva2|d3d11va/videotoolbox support is seen
as "alright"). Then, depending on the amount of working alternatives
that are under licenses which do not require additional limitations to
available configurations, and acceptance of the dependencies utilized,
we have components which require specific sets of configuration.
Examples of such would be:
- libaribb24 requires LGPLv3+ in its current git form, and GPLv3+ in
its current latest release form. There is an alternative, but it
requires porting of a custom glibc iconv module into the code base
first, so usage of the alternative is not realistic right away. Thus
one could argue that it might still be worth the while to have support
for this library in the code base.
- libfdk-aac is open source, and to my (admittedly hazy) understanding
the patent-related clause only affects GPL and not LGPL configurations
(due to the "no additional limitations" clause in the former?). There
are no open source alternatives providing similar level of quality for
HE-AAC, thus making it arguable that fdk-aac a worthwhile thing to
keep around.

Now, back to libNDI. Let's start with the side that in my opinion
looks more positive to it, and then move to the things where I see it
in a less positive way:

- libNDI does not have open source alternatives, which would be under
a license that could be used for a re-distributable binary.
- libNDI does have a use case and it could be argued that there is a
need for it in the community.

Just looking at these first two lines, I could still argue that it
might be worth it to have in the code base. But only if the basic
requirement regarding the dependency's licensing passes. libNDI's
state in my opinion is as follows:

- libNDI is closed source, and even according to the more generous
readings of the most strict license you can configure your FFmpeg
binary under (GPL) cannot be considered acceptable as it is not a
hardware driver interface, but rather just a network protocol
implementation.

Thus the general "is this OK" check for me does not get passed.
Decklink for example seems to pass since I see people being OK with
the hardware interface driver interpretation, and if I understand it
correctly the reason why it is under nonfree currently is due to the
SDK's poor licensing (?).

I would have loved to have had this discussion happen in 2017, before
libNDI support getting merged. That way this would not look like a
knee-jerk reaction to certain events during the last year or so. Alas,
that was not what happened. I am one of those guilty as charged for
one reason or another to have not replied into that thread. Maybe we
could have persuaded people to work on an open source alternative
implementation earlier, instead of now. Also this would have less
inconvenienced users who did have this wrapper in their FFmpeg code
b

Re: [FFmpeg-devel] [PATCH] lavc/videotoolboxenc: Fix compilation for IOS < 11.0 and OSX, < 10.13

2019-05-21 Thread Jan Ekström
Hi,

On Tue, May 14, 2019 at 4:16 PM Thilo Borgmann  wrote:
>
> $Subject
>
> Tested compilation only, sanity test actually using it appreciated.
>

Thanks for the patch. To be completely fair, this is not to fix
compilation for specific target systems, but rather to fix compilation
on older SDKs (building with newer SDKs you can still build aiming for
macOS starting from 10.9, for example).

I didn't notice a patch landed on the encoder side that utilized the
defines without further checking/ifdefs. Too bad. I think I
specifically didn't yet merge the full/limited range patch on the
decoder side due to related reasons.

I did notice that VLC just re-defined these enum values themselves to
stop needing to have ifdefs depending on which SDK is being utilized
(https://github.com/videolan/vlc/commit/1b7e1c4bfcda375e2d4e657135aeaf3732e44af2#diff-a11cdb805d111956af60619d7dfa022bR735).
I wonder if we should have a helper header that would re-define these
enum values with their name. That way the code would look correct, and
the resulting binary has the same features independent of the SDK it
had been built under.

What would be the opinion of people on a solution like this?

Best regards,
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".

Re: [FFmpeg-devel] [PATCH] lavf/tls_gnutls: retry gnutls_handshake on non fatal errors

2019-06-01 Thread Jan Ekström
On Wed, Mar 27, 2019 at 2:09 PM Remita Amine  wrote:
>
> fixes #7801
>
> Signed-off-by: Remita Amine 

This seems to fix switching the cipher suite, and quickly looking at
the gnutls API docs this seems to be the way to do it.

Just tested this with the following as I got a report that opening
Facebook HTTPS URLs didn't work in a libavformat API client:
1. youtube-dl -g "https://www.facebook.com/downshiftaus/videos/418766325569703/";
   (you receive a URL)
2. ffprobe 'THAT_URL'

Without this patch the handshake fails (as there is a cipher
re-negotiation?), and with the patch it works.

Additionally, this doesn't seem to enable bad TLS configurations such
https://rc4.badssl.com/ to get opened. Which is expected from the
gnutls docs, but still I felt like testing.

In other words, I think this is LGTM and since there already are
reports from people on distros running into this, this should be
back-ported to the versions still maintained by us (whatever those
are) ?

Best regards,
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".

Re: [FFmpeg-devel] VP9 RTP encoder and WebRTC

2019-06-01 Thread Jan Ekström
Hi,

On Sat, Jun 1, 2019 at 8:35 PM Alex Protasenko  wrote:
>
> Hello,
>
>
> I'm trying to play some realtime video sources (web/IP cam) using WebRTC
> in a browser. I'm sending RTP stream via Janus gateway using VP9 codec,
> hardware transcoded using ffmpeg.
>
> Everything works fine except random frame corruption happening around
> moving objects, portions of frame "flowing off the screen" and such
> until next keyframe fixes it. This happens consistently especially at
> higher framerates.
>
> It turns out the issue  could be narrowed down to the VP9 RTP
> packetizer. The problem is it's not marking P frames vs I frames in the
> VP9 payload descriptor octet (the P bit). Gstreamer does that and
> doesn't experience any such corruption issues.
>
> I added this simple change and now WebRTC plays any stream 100% solid
> and corruption free for me.
>
>
> Could somebody implement this simple fix in the upstream. Basically
> in libavformat/rtpenc_vp9.c add something to the effect of the following
> two lines (to set the P bit for all but I frames):
>
>  /* mark the first fragment */
>  *rtp_ctx->buf_ptr++ = 0x08;
>
> +if (!keyframe) {
> +rtp_ctx->buf[0] |= 0x40;
>
>
> Where the "keyframe" is an additional boolean parameter to the
> ff_rtp_send_vp9 function which could be called as:
>
> ff_rtp_send_vp9(s1, pkt->data, size, pkt->flags & AV_PKT_FLAG_KEY);
>

Is https://tools.ietf.org/html/draft-ietf-payload-vp9-06#page-7 the
correct specification for VP9 in RTP?

Best regards,
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".

  1   2   3   4   5   6   7   8   9   10   >