Re: [FFmpeg-devel] [PATCH v4] avformat: add MMTP parser and MMT/TLV demuxer

2023-04-29 Thread SuperFashi
Thanks. Is there anything else that does not fit codebase tradition? Please
point everything out so I could send a new patch.

On Sun, Apr 30, 2023 at 3:06 AM Michael Niedermayer 
wrote:

> On Sat, Apr 29, 2023 at 02:53:06PM +0900, SuperFashi wrote:
> > v1 -> v2: Refactor using GetByteContext; Fix compile error.
> > v2 -> v3: Remove debug statement.
> > v3 -> v4: Squash commits (sorry, first time git patch user :(
> >
> > This patch adds an MPEG Media Transport Protocol (MMTP) parser, as
> defined in ISO/IEC 23008-1, and an MMT protocol over TLV packets (MMT/TLV)
> demuxer, as defined in ARIB STD-B32. Currently, it supports HEVC, AAC LATM,
> and ARIB-TTML demuxing.
> >
> > Since MMTP is designed to transmit over IP, there is no size information
> within each MMTP packet, and there is no filesystem format defined
> alongside the protocol. One industrial solution is a simple container
> format using type–length–value packets, which is defined in ARIB STD-B32.
> >
> > Another known container format for MMTP is using packet capture (pcap)
> files which records network packets. This patch does not include the
> demuxer for this container format.
> >
> > Signed-off-by: SuperFashi 
> > ---
> >  Changelog|1 +
> >  doc/demuxers.texi|4 +
> >  libavformat/Makefile |1 +
> >  libavformat/allformats.c |1 +
> >  libavformat/mmtp.c   | 1372 ++
> >  libavformat/mmtp.h   |   61 ++
> >  libavformat/mmttlv.c |  324 +
> >  libavformat/version.h|2 +-
> >  8 files changed, 1765 insertions(+), 1 deletion(-)
> >  create mode 100644 libavformat/mmtp.c
> >  create mode 100644 libavformat/mmtp.h
> >  create mode 100644 libavformat/mmttlv.c
> >
> > diff --git a/Changelog b/Changelog
> > index b6f6682904..2483fdd547 100644
> > --- a/Changelog
> > +++ b/Changelog
> > @@ -6,6 +6,7 @@ version :
> >  - Playdate video decoder and demuxer
> >  - Extend VAAPI support for libva-win32 on Windows
> >  - afireqsrc audio source filter
> > +- MMTP parser and MMT/TLV demuxer
> >
> >  version 6.0:
> >  - Radiance HDR image support
> > diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> > index 2d33b47a56..56aab251b2 100644
> > --- a/doc/demuxers.texi
> > +++ b/doc/demuxers.texi
> > @@ -689,6 +689,10 @@ Set the sample rate for libopenmpt to output.
> >  Range is from 1000 to INT_MAX. The value default is 48000.
> >  @end table
> >
> > +@section mmttlv
> > +
> > +Demuxer for MMT protocol over TLV packets (MMT/TLV), as defined in ARIB
> STD-B32.
> > +
> >  @section mov/mp4/3gp
> >
> >  Demuxer for Quicktime File Format & ISO/IEC Base Media File Format
> (ISO/IEC 14496-12 or MPEG-4 Part 12, ISO/IEC 15444-12 or JPEG 2000 Part 12).
> > diff --git a/libavformat/Makefile b/libavformat/Makefile
> > index f8ad7c6a11..e32d6e71a3 100644
> > --- a/libavformat/Makefile
> > +++ b/libavformat/Makefile
> > @@ -354,6 +354,7 @@ OBJS-$(CONFIG_MLV_DEMUXER)   += mlvdec.o
> riffdec.o
> >  OBJS-$(CONFIG_MM_DEMUXER)+= mm.o
> >  OBJS-$(CONFIG_MMF_DEMUXER)   += mmf.o
> >  OBJS-$(CONFIG_MMF_MUXER) += mmf.o rawenc.o
> > +OBJS-$(CONFIG_MMTTLV_DEMUXER)+= mmtp.o mmttlv.o
> >  OBJS-$(CONFIG_MODS_DEMUXER)  += mods.o
> >  OBJS-$(CONFIG_MOFLEX_DEMUXER)+= moflex.o
> >  OBJS-$(CONFIG_MOV_DEMUXER)   += mov.o mov_chan.o mov_esds.o
> \
> > diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> > index efdb34e29d..d5f4f5680e 100644
> > --- a/libavformat/allformats.c
> > +++ b/libavformat/allformats.c
> > @@ -270,6 +270,7 @@ extern const AVInputFormat  ff_mlv_demuxer;
> >  extern const AVInputFormat  ff_mm_demuxer;
> >  extern const AVInputFormat  ff_mmf_demuxer;
> >  extern const FFOutputFormat ff_mmf_muxer;
> > +extern const AVInputFormat  ff_mmttlv_demuxer;
> >  extern const AVInputFormat  ff_mods_demuxer;
> >  extern const AVInputFormat  ff_moflex_demuxer;
> >  extern const AVInputFormat  ff_mov_demuxer;
> > diff --git a/libavformat/mmtp.c b/libavformat/mmtp.c
> > new file mode 100644
> > index 00..ba1fcab281
> > --- /dev/null
> > +++ b/libavformat/mmtp.c
> > @@ -0,0 +1,1372 @@
> > +/*
> > + * MPEG Media Transport Protocol (MMTP) parser, as defined in ISO/IEC
> 23008-1.
> > + * Copyright (c) 2023 SuperFashi
> > + *
> > + * 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 

Re: [FFmpeg-devel] [PATCH 5/8] lavu: add a JSON writer API (WIP)

2023-04-29 Thread Kieran Kunhya
On Sat, 29 Apr 2023 at 05:07, Derek Buitenhuis 
wrote:

> On 4/29/2023 10:41 AM, Anton Khirnov wrote:
> > ffprobe:
> > * is not one of the libraries, but rather their caller
> > * we are not in business of providing random non-multimedia-related
> >   services to callers, unless they are useful in our libraries;
> >   if this code is only useful in ffprobe then it should live in fftools/
> > * it is not at all obvious that switching ffprobe to this code would
> >   be an improvement; a patch actually demonstrating this would be most
> >   useful
>
> +1
>
>
+2
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


Re: [FFmpeg-devel] [PATCH 1/5] avutil/frame: add new interlaced and top_field_first flags

2023-04-29 Thread James Almer

On 4/12/2023 4:49 PM, James Almer wrote:

Signed-off-by: James Almer 
---
Missing version bump and APIChanges entry.

  libavutil/frame.h | 9 +
  1 file changed, 9 insertions(+)

diff --git a/libavutil/frame.h b/libavutil/frame.h
index 5b58c14ac3..87e0a51226 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -586,6 +586,15 @@ typedef struct AVFrame {
   * A flag to mark the frames which need to be decoded, but shouldn't be 
output.
   */
  #define AV_FRAME_FLAG_DISCARD   (1 << 2)
+/**
+ * A flag to mark frames whose content is interlaced.
+ */
+#define AV_FRAME_FLAG_INTERLACED (1 << 3)
+/**
+ * A flag to mark frames where the top field is displayed first if the content
+ * is interlaced.
+ */
+#define AV_FRAME_FLAG_TOP_FIELD_FIRST (1 << 4)
  /**
   * @}
   */


If no one objects, I'll push this set and the key_frame one this week.
___
ffmpeg-devel mailing list
ffmpeg-devel@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/parser: fill avctx dimensions if unset

2023-04-29 Thread James Almer

On 4/27/2023 12:39 PM, James Almer wrote:

This allows the usage of codecs in builds that have a parser but no decoders
for remuxing scenarios with raw sources.

Signed-off-by: James Almer 
---
  libavcodec/parser.c | 4 
  1 file changed, 4 insertions(+)

diff --git a/libavcodec/parser.c b/libavcodec/parser.c
index 49de7e6a57..efc28b8918 100644
--- a/libavcodec/parser.c
+++ b/libavcodec/parser.c
@@ -166,6 +166,10 @@ int av_parser_parse2(AVCodecParserContext *s, 
AVCodecContext *avctx,
  #define FILL(name) if(s->name > 0 && avctx->name <= 0) avctx->name = s->name
  if (avctx->codec_type == AVMEDIA_TYPE_VIDEO) {
  FILL(field_order);
+FILL(coded_width);
+FILL(coded_height);
+FILL(width);
+FILL(height);
  }
  
  /* update the file pointer */


Will apply.
___
ffmpeg-devel mailing list
ffmpeg-devel@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 5/8] lavu: add a JSON writer API (WIP)

2023-04-29 Thread Nicolas George
Anton Khirnov (12023-04-29):
> libavfilter is a C library with a C API. Any structured output from
> filters should be in the form of a C object, typically a struct. I do
> not see why are you so in love with strings, they make for terrible
> APIs.

Yes, strings are a terrible API, but our project is not only a set of
libraries, it is also a set of command-line tools, and command-line
tools work with strings and nothing else.

-- 
  Nicolas George


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

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


Re: [FFmpeg-devel] [PATCH 5/8] lavu: add a JSON writer API (WIP)

2023-04-29 Thread Anton Khirnov
Quoting Nicolas George (2023-04-29 19:11:52)
> Anton Khirnov (2023-04-29):
> > As far as I can see, there are exactly two places in the codebase that
> > produce JSON: af_loudnorm and ffprobe.
> 
> I think I remember finding a few other places, but it does not matter
> much.
> 
> Users have been asking¹ for parse-friendly output from filters for years,
> and JSON is often the format they prefer. Therefore, all the filters
> that log useful information are candidates where a JSON writing API is
> useful.

libavfilter is a C library with a C API. Any structured output from
filters should be in the form of a C object, typically a struct. I do
not see why are you so in love with strings, they make for terrible
APIs.

We could conceivably use e.g. the AVOption mechanism to allow automated
serialization of such structs, but the actual mechanism should be left
to the callers.

> (See my answer to James for why “put it in frame metadata” is not a real
> solution.)

I believe frame metadata is a mistake and should not exist, so I am
certainly not suggesting to use it for anything.

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@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] avformat: add MMTP parser and MMT/TLV demuxer

2023-04-29 Thread Michael Niedermayer
On Sat, Apr 29, 2023 at 02:53:06PM +0900, SuperFashi wrote:
> v1 -> v2: Refactor using GetByteContext; Fix compile error.
> v2 -> v3: Remove debug statement.
> v3 -> v4: Squash commits (sorry, first time git patch user :(
> 
> This patch adds an MPEG Media Transport Protocol (MMTP) parser, as defined in 
> ISO/IEC 23008-1, and an MMT protocol over TLV packets (MMT/TLV) demuxer, as 
> defined in ARIB STD-B32. Currently, it supports HEVC, AAC LATM, and ARIB-TTML 
> demuxing.
> 
> Since MMTP is designed to transmit over IP, there is no size information 
> within each MMTP packet, and there is no filesystem format defined alongside 
> the protocol. One industrial solution is a simple container format using 
> type–length–value packets, which is defined in ARIB STD-B32.
> 
> Another known container format for MMTP is using packet capture (pcap) files 
> which records network packets. This patch does not include the demuxer for 
> this container format.
> 
> Signed-off-by: SuperFashi 
> ---
>  Changelog|1 +
>  doc/demuxers.texi|4 +
>  libavformat/Makefile |1 +
>  libavformat/allformats.c |1 +
>  libavformat/mmtp.c   | 1372 ++
>  libavformat/mmtp.h   |   61 ++
>  libavformat/mmttlv.c |  324 +
>  libavformat/version.h|2 +-
>  8 files changed, 1765 insertions(+), 1 deletion(-)
>  create mode 100644 libavformat/mmtp.c
>  create mode 100644 libavformat/mmtp.h
>  create mode 100644 libavformat/mmttlv.c
> 
> diff --git a/Changelog b/Changelog
> index b6f6682904..2483fdd547 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -6,6 +6,7 @@ version :
>  - Playdate video decoder and demuxer
>  - Extend VAAPI support for libva-win32 on Windows
>  - afireqsrc audio source filter
> +- MMTP parser and MMT/TLV demuxer
>  
>  version 6.0:
>  - Radiance HDR image support
> diff --git a/doc/demuxers.texi b/doc/demuxers.texi
> index 2d33b47a56..56aab251b2 100644
> --- a/doc/demuxers.texi
> +++ b/doc/demuxers.texi
> @@ -689,6 +689,10 @@ Set the sample rate for libopenmpt to output.
>  Range is from 1000 to INT_MAX. The value default is 48000.
>  @end table
>  
> +@section mmttlv
> +
> +Demuxer for MMT protocol over TLV packets (MMT/TLV), as defined in ARIB 
> STD-B32.
> +
>  @section mov/mp4/3gp
>  
>  Demuxer for Quicktime File Format & ISO/IEC Base Media File Format (ISO/IEC 
> 14496-12 or MPEG-4 Part 12, ISO/IEC 15444-12 or JPEG 2000 Part 12).
> diff --git a/libavformat/Makefile b/libavformat/Makefile
> index f8ad7c6a11..e32d6e71a3 100644
> --- a/libavformat/Makefile
> +++ b/libavformat/Makefile
> @@ -354,6 +354,7 @@ OBJS-$(CONFIG_MLV_DEMUXER)   += mlvdec.o 
> riffdec.o
>  OBJS-$(CONFIG_MM_DEMUXER)+= mm.o
>  OBJS-$(CONFIG_MMF_DEMUXER)   += mmf.o
>  OBJS-$(CONFIG_MMF_MUXER) += mmf.o rawenc.o
> +OBJS-$(CONFIG_MMTTLV_DEMUXER)+= mmtp.o mmttlv.o
>  OBJS-$(CONFIG_MODS_DEMUXER)  += mods.o
>  OBJS-$(CONFIG_MOFLEX_DEMUXER)+= moflex.o
>  OBJS-$(CONFIG_MOV_DEMUXER)   += mov.o mov_chan.o mov_esds.o \
> diff --git a/libavformat/allformats.c b/libavformat/allformats.c
> index efdb34e29d..d5f4f5680e 100644
> --- a/libavformat/allformats.c
> +++ b/libavformat/allformats.c
> @@ -270,6 +270,7 @@ extern const AVInputFormat  ff_mlv_demuxer;
>  extern const AVInputFormat  ff_mm_demuxer;
>  extern const AVInputFormat  ff_mmf_demuxer;
>  extern const FFOutputFormat ff_mmf_muxer;
> +extern const AVInputFormat  ff_mmttlv_demuxer;
>  extern const AVInputFormat  ff_mods_demuxer;
>  extern const AVInputFormat  ff_moflex_demuxer;
>  extern const AVInputFormat  ff_mov_demuxer;
> diff --git a/libavformat/mmtp.c b/libavformat/mmtp.c
> new file mode 100644
> index 00..ba1fcab281
> --- /dev/null
> +++ b/libavformat/mmtp.c
> @@ -0,0 +1,1372 @@
> +/*
> + * MPEG Media Transport Protocol (MMTP) parser, as defined in ISO/IEC 
> 23008-1.
> + * Copyright (c) 2023 SuperFashi
> + *
> + * This file is part of FFmpeg.
> + *
> + * FFmpeg is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2.1 of the License, or (at your option) any later version.
> + *
> + * FFmpeg is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with FFmpeg; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 
> USA
> + */
> +#include "libavutil/mem.h"
> +#include "libavutil/avassert.h"
> +#include "libavutil/intreadwrite.h"
> +#include "libavcodec/bytestream.h"
> +#include "network.h"
> +#include "mmtp.h"

Re: [FFmpeg-devel] [PATCH 5/8] lavu: add a JSON writer API (WIP)

2023-04-29 Thread Nicolas George
James Almer (12023-04-29):
> It should be exporting frame metadata

Not really. Exporting information to frame data is a hack, “this looks
pointy, we have a hammer, let's nail it”.

It only works if the data is very flat.

It does not work for data that comes at the end of the stream.

It still requires a way to get the data out in a parse-friendly syntax
when the program running is not ffprobe.

Having an API and user interface for filters to return structured data
is one of the reasons I want this API in libavutil.

Regards,

-- 
  Nicolas George


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

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


Re: [FFmpeg-devel] [PATCH 5/8] lavu: add a JSON writer API (WIP)

2023-04-29 Thread Nicolas George
Anton Khirnov (12023-04-29):
> As far as I can see, there are exactly two places in the codebase that
> produce JSON: af_loudnorm and ffprobe.

I think I remember finding a few other places, but it does not matter
much.

Users have been asking¹ for parse-friendly output from filters for years,
and JSON is often the format they prefer. Therefore, all the filters
that log useful information are candidates where a JSON writing API is
useful.

(See my answer to James for why “put it in frame metadata” is not a real
solution.)

> * IMO the filter should not be doing this at all and instead produce some
>   sort of a struct and let the users process it as they wish

And to let users process it as they wish, it needs to be formatted into
a parse-friendly syntax like JSON.

> ffprobe:
> * is not one of the libraries, but rather their caller
> * we are not in business of providing random non-multimedia-related
>   services to callers, unless they are useful in our libraries;
>   if this code is only useful in ffprobe then it should live in fftools/

This is your opinion, not the project policy.

My opinion is that anything that is useful for fftools and can be made
to have a properly-defined API has its place in libavutil.

Let us see which opinion has more support.

> * it is not at all obvious that switching ffprobe to this code would
>   be an improvement; a patch actually demonstrating this would be most
>   useful

I distinctly remember writing the changes to ffprobe and getting the
same output except for the indentation level, but I do not find the
code.


1: To be aware what would be useful to the project, reading the users
mailing-lists helps.

-- 
  Nicolas George


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

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


Re: [FFmpeg-devel] [PATCH 08/21] fftools/ffmpeg_filter: add filtergraph private data

2023-04-29 Thread Nicolas George
James Almer (12023-04-29):
> History has shown that these notifications have had no effect on users, and
> even on this same project's developers. A good example was ffserver.c, which
> accessed an unholy amount of lavf private fields (both exposed in public
> headers and even internal ones), and first_dts from AVStream, which was not
> only accessed by prominent library users (One of which refuses to do things
> right and forces distros to use a patch to expose said field on their ffmpeg
> packages for the sake of supporting their application, thus making a pure
> recompilation from our tree no longer a drop-in solution on anyone's
> system), but also by ffmpeg.c, with no developer noticing it to prevent such
> code being pushed.

Since we are discussing types for the fftools, not librairies, what
people outside the project might do is not an issue

As for abuse of fields private to a part of the code by another part of
the code, I think you are somewhat rewriting history here.

The reason the fftools used to access private fields of the libraries is
not that developers neglected the rules against using them, it is that
no such rule existed when the code was written, they came later.

But if you really think we need to enforce the rule, then there still
are better solutions than using a separate structure and littering the
code with casts.

For example we can apply __attribute__((unavailable)) to the private
fields except in the part of the code where they are allowed.

(The attribute might not be supported on all compilers, but it is plenty
enough that it is supported on the compilers used by the FATE instances
that test submitted patches and the compilers used by some of us.)

This is a MUCH better solution than what is proposed here.

Regards,

-- 
  Nicolas George


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

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


Re: [FFmpeg-devel] [PATCH 1/8] lavu: add macros to help making future-proof structures

2023-04-29 Thread Derek Buitenhuis
On 4/29/2023 9:17 AM, Anton Khirnov wrote:
> What important problem within the scope of the project is being solved
> by this? Why do we need over a 1000 lines of just new header files for
> it? Why do we need a generic JSON writer? We are not a JSON library.
> Neither are we a string processing library.

I agree, I don't think it has a place in a set of multimedia libraries.

If I wanted this functionality there are many other good libaries I could
use that also don't pull in a multimeia framework.

> IMO this should not go in until and unless significant practical
> benefits of this code are shown on a non-toy use case within the scope
> of our project.

Having watched this from afar for a while it seems like a classic FOSS
problem:

One person may have infinite energy to argue ther point / idea until
nobody can be bothered to reply anymore since it is so draining.

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

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


Re: [FFmpeg-devel] [PATCH 5/8] lavu: add a JSON writer API (WIP)

2023-04-29 Thread Derek Buitenhuis
On 4/29/2023 10:41 AM, Anton Khirnov wrote:
> ffprobe:
> * is not one of the libraries, but rather their caller
> * we are not in business of providing random non-multimedia-related
>   services to callers, unless they are useful in our libraries;
>   if this code is only useful in ffprobe then it should live in fftools/
> * it is not at all obvious that switching ffprobe to this code would
>   be an improvement; a patch actually demonstrating this would be most
>   useful

+1

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

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


Re: [FFmpeg-devel] [PATCH 5/8] lavu: add a JSON writer API (WIP)

2023-04-29 Thread James Almer

On 4/29/2023 6:41 AM, Anton Khirnov wrote:

Quoting Nicolas George (2023-04-29 11:11:59)

Nicolas George (12023-04-28):

Signed-off-by: Nicolas George 
---
  libavutil/Makefile |   1 +
  libavutil/json.c   | 368 +++
  libavutil/json.h   | 470 +
  3 files changed, 839 insertions(+)
  create mode 100644 libavutil/json.c
  create mode 100644 libavutil/json.h


I forgot to write: I wrote this code not only because we have half-baked
JSON output in multiple places in the code


As far as I can see, there are exactly two places in the codebase that
produce JSON: af_loudnorm and ffprobe.

af_loudnorm:
* can optionally produce final filter stats as JSON output with av_log()
* the relevant code has ~25 lines and is unlikely to be simplified by
   this
* IMO the filter should not be doing this at all and instead produce some
   sort of a struct and let the users process it as they wish


It should be exporting frame metadata, like aphasemeter, cropdetect, etc.
av_log() output is not meant to be parseable. It's why the relevant 
output of vf_showinfo can be freely changed, whereas output from things 
like framecrc/framehash muxers is standardized.




ffprobe:
* is not one of the libraries, but rather their caller
* we are not in business of providing random non-multimedia-related
   services to callers, unless they are useful in our libraries;
   if this code is only useful in ffprobe then it should live in fftools/
* it is not at all obvious that switching ffprobe to this code would
   be an improvement; a patch actually demonstrating this would be most
   useful


___
ffmpeg-devel mailing list
ffmpeg-devel@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 08/21] fftools/ffmpeg_filter: add filtergraph private data

2023-04-29 Thread James Almer

On 4/29/2023 6:13 AM, Nicolas George wrote:

Anton Khirnov (12023-04-29):

That does not follow at all - just because something was moved once does
not mean it cannot be moved again if a better place is found later.


Good, then you should have no objection to replacing your private
structure with a “/* these fields are private to… */” comment.


History has shown that these notifications have had no effect on users, 
and even on this same project's developers. A good example was 
ffserver.c, which accessed an unholy amount of lavf private fields (both 
exposed in public headers and even internal ones), and first_dts from 
AVStream, which was not only accessed by prominent library users (One of 
which refuses to do things right and forces distros to use a patch to 
expose said field on their ffmpeg packages for the sake of supporting 
their application, thus making a pure recompilation from our tree no 
longer a drop-in solution on anyone's system), but also by ffmpeg.c, 
with no developer noticing it to prevent such code being pushed.


So no, i don't support a simple "please, don't touch the shinny and 
enticing object in the table" notifications. If in the future this 
approach here is replaced, it needs to be by something better/cleaner 
that also keeps things local.

___
ffmpeg-devel mailing list
ffmpeg-devel@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: add MMTP parser and MMT/TLV demuxer

2023-04-29 Thread SuperFashi
Please see new version.

On Sat, Apr 29, 2023 at 20:38 Jean-Baptiste Kempf  wrote:

> On Sat, 29 Apr 2023, at 07:44, SuperFashi wrote:
> > +#define AVERROR_INVALIDDATA (abort(), 0)
>
> Why are you aborting?
>
> --
> Jean-Baptiste Kempf -  President
> +33 672 704 734
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
>
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

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


Re: [FFmpeg-devel] [PATCH v2] avformat: add MMTP parser and MMT/TLV demuxer

2023-04-29 Thread Jean-Baptiste Kempf
On Sat, 29 Apr 2023, at 07:44, SuperFashi wrote:
> +#define AVERROR_INVALIDDATA (abort(), 0)

Why are you aborting?

-- 
Jean-Baptiste Kempf -  President
+33 672 704 734
___
ffmpeg-devel mailing list
ffmpeg-devel@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 5/8] lavu: add a JSON writer API (WIP)

2023-04-29 Thread Anton Khirnov
Quoting Nicolas George (2023-04-29 11:11:59)
> Nicolas George (12023-04-28):
> > Signed-off-by: Nicolas George 
> > ---
> >  libavutil/Makefile |   1 +
> >  libavutil/json.c   | 368 +++
> >  libavutil/json.h   | 470 +
> >  3 files changed, 839 insertions(+)
> >  create mode 100644 libavutil/json.c
> >  create mode 100644 libavutil/json.h
> 
> I forgot to write: I wrote this code not only because we have half-baked
> JSON output in multiple places in the code

As far as I can see, there are exactly two places in the codebase that
produce JSON: af_loudnorm and ffprobe.

af_loudnorm:
* can optionally produce final filter stats as JSON output with av_log()
* the relevant code has ~25 lines and is unlikely to be simplified by
  this
* IMO the filter should not be doing this at all and instead produce some
  sort of a struct and let the users process it as they wish

ffprobe:
* is not one of the libraries, but rather their caller
* we are not in business of providing random non-multimedia-related
  services to callers, unless they are useful in our libraries;
  if this code is only useful in ffprobe then it should live in fftools/
* it is not at all obvious that switching ffprobe to this code would
  be an improvement; a patch actually demonstrating this would be most
  useful

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@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 08/21] fftools/ffmpeg_filter: add filtergraph private data

2023-04-29 Thread Nicolas George
Anton Khirnov (12023-04-29):
> That does not follow at all - just because something was moved once does
> not mean it cannot be moved again if a better place is found later.

Good, then you should have no objection to replacing your private
structure with a “/* these fields are private to… */” comment.

-- 
  Nicolas George


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

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


Re: [FFmpeg-devel] [PATCH 5/8] lavu: add a JSON writer API (WIP)

2023-04-29 Thread Nicolas George
Nicolas George (12023-04-28):
> Signed-off-by: Nicolas George 
> ---
>  libavutil/Makefile |   1 +
>  libavutil/json.c   | 368 +++
>  libavutil/json.h   | 470 +
>  3 files changed, 839 insertions(+)
>  create mode 100644 libavutil/json.c
>  create mode 100644 libavutil/json.h

I forgot to write: I wrote this code not only because we have half-baked
JSON output in multiple places in the code, but also to show the
kind of API AVWriter makes possible.

Typical JSON APIs would have a function to write a string value or an
object key, requiring the caller to build the string beforehand. Of
course, this API can do that:

> +void av_json_add_string(AVJson *jc, const char *str);

including with a format string, which is less usual:

> +void av_json_add_string_printf(AVJson *jc, const char *fmt, ...) 
> av_printf_format(2, 3);

But these are just wrappers for convenience over the real API:

> +AVWriter av_json_begin_string(AVJson *jc);

It starts a string, i.e. outputs a quote, and then we get a writer to
write into that string. It will be automatically escaped, no
intermediate buffer will be used, and all the functions of the writer
API are available, including all the av_something_write() to come to
serialize our various types.

Also note that this API as a whole can produce small JSON outputs
without any dynamic allocation, making it suitable for once-per-frame
calls, but is not limited to that.

Do we *need* that: of course not, we still put “len += snprintf()” and
“av_realloc()” and error checks all over the place.

Now, while people maybe look at the code, there are a few things I can
work on, and I wonder which one you would like to see first:

- Finishing this JSON API.

- A XML API: that would be useful for dashenc, movenc, ttmlenc, ffprobe
  and possibly others.

- Add serialization functions for our various types, like I did for
  av_disposition_write().

- Go forward with the others API enhancements that I promised and that
  depend on AVWriter.

Regards,

-- 
  Nicolas George


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

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


Re: [FFmpeg-devel] [PATCH 08/21] fftools/ffmpeg_filter: add filtergraph private data

2023-04-29 Thread Anton Khirnov
Quoting Nicolas George (2023-04-29 10:46:31)
> Nicolas George (12023-04-28):
> > And during the work of turning all into threads, opportunities to split
> > the structure in more logical ways with less code noise will almost
> > certainly present themselves.
> 
> I had intended to not reply further on this, but I just had a severe
> case of esprit de l'ecalier, so I just add this:
> 
> For example, it is entirely possible that a lot of these “private”
> fields could become local variables and functions parameters in the
> threads. That would be elegant and efficient.
> 
> But now we will never know, because they will all be stuffed in a
> private context, according to “good practices”, and that will spare the
> effort of checking what their exact scope is.

That does not follow at all - just because something was moved once does
not mean it cannot be moved again if a better place is found later.

This change is an incremental improvement, I am not claiming it is the
best possible shape this code could ever have. In my experience, it is
much better to have a series of actually performed incremental
improvements, than to bikeshed endlessly about what the perfect final
endstate should be and never get anything done.

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@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 08/21] fftools/ffmpeg_filter: add filtergraph private data

2023-04-29 Thread Nicolas George
Nicolas George (12023-04-28):
> And during the work of turning all into threads, opportunities to split
> the structure in more logical ways with less code noise will almost
> certainly present themselves.

I had intended to not reply further on this, but I just had a severe
case of esprit de l'ecalier, so I just add this:

For example, it is entirely possible that a lot of these “private”
fields could become local variables and functions parameters in the
threads. That would be elegant and efficient.

But now we will never know, because they will all be stuffed in a
private context, according to “good practices”, and that will spare the
effort of checking what their exact scope is.

Regards,

-- 
  Nicolas George


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

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


Re: [FFmpeg-devel] [PATCH 1/8] lavu: add macros to help making future-proof structures

2023-04-29 Thread Anton Khirnov
I've said this before, repeating it again for ease of reference: I do
not believe the use case for this has been sufficiently established.

What important problem within the scope of the project is being solved
by this? Why do we need over a 1000 lines of just new header files for
it? Why do we need a generic JSON writer? We are not a JSON library.
Neither are we a string processing library.

IMO this should not go in until and unless significant practical
benefits of this code are shown on a non-toy use case within the scope
of our project.

-- 
Anton Khirnov
___
ffmpeg-devel mailing list
ffmpeg-devel@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] fftools/ffmpeg: rework audio-decode timestamp handling

2023-04-29 Thread Anton Khirnov
Stop using InputStream.dts for generating missing timestamps for decoded
frames, because it contains pre-decoding timestamps and there may be
arbitrary amount of delay between input packets and output frames (e.g.
dependent on the thread count when frame threading is used). It is also
in AV_TIME_BASE (i.e. microseconds), which may introduce unnecessary
rounding issues.

New code maintains a timebase that is the inverse of the LCM of all the
samplerates seen so far, and thus can accurately represent every audio
sample. This timebase is used to generate missing timestamps after
decoding.

Changes the result of the following FATE tests
* pcm_dvd-16-5.1-96000
* lavf-smjpeg
* adpcm-ima-smjpeg
In all of these the timestamps now better correspond to actual frame
durations.
---
The branch can also be downloaded as 'ffmpeg_audio_ts' in my tree.
---
 fftools/ffmpeg.c|  98 -
 fftools/ffmpeg.h|   8 +-
 fftools/ffmpeg_demux.c  |   6 +-
 tests/ref/fate/adpcm-ima-smjpeg | 658 ++--
 tests/ref/fate/pcm_dvd-16-5.1-96000 |   8 +-
 tests/ref/lavf/smjpeg   |   2 +-
 6 files changed, 423 insertions(+), 357 deletions(-)

diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 8829a163e0..8dcc70e879 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -881,6 +881,85 @@ static int send_frame_to_filters(InputStream *ist, AVFrame 
*decoded_frame)
 return ret;
 }
 
+static AVRational audio_samplerate_update(InputStream *ist, const AVFrame 
*frame)
+{
+const int prev = ist->last_frame_tb.den;
+const int sr   = frame->sample_rate;
+
+AVRational tb_new;
+int64_t gcd;
+
+if (frame->sample_rate == ist->last_frame_sample_rate)
+goto finish;
+
+gcd  = av_gcd(prev, sr);
+
+if (prev / gcd >= INT_MAX / sr) {
+av_log(ist, AV_LOG_WARNING,
+   "Audio timestamps cannot be represented exactly after "
+   "sample rate change: %d -> %d\n", prev, sr);
+
+// LCM of 192000, 44100, allows to represent all common samplerates
+tb_new = (AVRational){ 1, 28224000 };
+} else
+tb_new = (AVRational){ 1, prev / gcd * sr };
+
+// keep the frame timebase if it is strictly better than
+// the samplerate-defined one
+if (frame->time_base.num == 1 && frame->time_base.den > tb_new.den &&
+!(frame->time_base.den % tb_new.den))
+tb_new = frame->time_base;
+
+if (ist->last_frame_pts != AV_NOPTS_VALUE)
+ist->last_frame_pts = av_rescale_q(ist->last_frame_pts,
+   ist->last_frame_tb, tb_new);
+ist->last_frame_duration_est = av_rescale_q(ist->last_frame_duration_est,
+ist->last_frame_tb, tb_new);
+
+ist->last_frame_tb  = tb_new;
+ist->last_frame_sample_rate = frame->sample_rate;
+
+finish:
+return ist->last_frame_tb;
+}
+
+static void audio_ts_process(InputStream *ist, AVFrame *frame)
+{
+AVRational tb_filter = (AVRational){1, frame->sample_rate};
+AVRational tb;
+int64_t pts_pred;
+
+// on samplerate change, choose a new internal timebase for timestamp
+// generation that can represent timestamps from all the samplerates
+// seen so far
+tb = audio_samplerate_update(ist, frame);
+pts_pred = ist->last_frame_pts == AV_NOPTS_VALUE ? 0 :
+   ist->last_frame_pts + ist->last_frame_duration_est;
+
+if (frame->pts == AV_NOPTS_VALUE) {
+frame->pts = pts_pred;
+frame->time_base = tb;
+} else if (ist->last_frame_pts != AV_NOPTS_VALUE &&
+   frame->pts > av_rescale_q_rnd(pts_pred, tb, frame->time_base,
+ AV_ROUND_UP)) {
+// there was a gap in timestamps, reset conversion state
+ist->filter_in_rescale_delta_last = AV_NOPTS_VALUE;
+}
+
+frame->pts = av_rescale_delta(frame->time_base, frame->pts,
+  tb, frame->nb_samples,
+  >filter_in_rescale_delta_last, tb);
+
+ist->last_frame_pts  = frame->pts;
+ist->last_frame_duration_est = av_rescale_q(frame->nb_samples,
+tb_filter, tb);
+
+// finally convert to filtering timebase
+frame->pts   = av_rescale_q(frame->pts, tb, tb_filter);
+frame->duration  = frame->nb_samples;
+frame->time_base = tb_filter;
+}
+
 static int decode_audio(InputStream *ist, AVPacket *pkt, int *got_output,
 int *decode_failed)
 {
@@ -910,23 +989,7 @@ static int decode_audio(InputStream *ist, AVPacket *pkt, 
int *got_output,
 ist->next_dts += ((int64_t)AV_TIME_BASE * decoded_frame->nb_samples) /
  decoded_frame->sample_rate;
 
-if (decoded_frame->pts == AV_NOPTS_VALUE) {
-decoded_frame->pts = ist->dts;
-decoded_frame->time_base = AV_TIME_BASE_Q;
-}
-if (pkt && 

Re: [FFmpeg-devel] I have an issue when try to using h264_amf encoding rtsp stream

2023-04-29 Thread Dmitrii Ovchinnikov
could you also try adding one more parameter and try again?

*  -header_spacing 30*it should have the same value as  -g in commandline.
For example:  *ffmpeg -i input.mkv  -c:v h264_amf  -g 30 -header_spacing 30
output.mp4*

ср, 19 апр. 2023 г. в 07:15, 罗勇刚(Yonggang Luo) :

> thanks, I have already tried the top size 1 and 10 before on the enconder,
> I assume you are talking about the encoder, as the decoder can not config
> the gop size, I think it's should be a bug of and encoder or decoder,
> because if I start the decoder before amf encoder, it's working fine, it is
> either because the encoder only encode the first I frame properly or the
> decoder can not recognize the I frame generated by amf encoder.
>
> Dmitrii Ovchinnikov  于 2023年4月19日周三
> 01:55写道:
>
> > For libx264 the default gop size is 10, while for amf the default gop
> size
> > is 250. I suspect this is the reason.
> > Try adding the following parameter  “* -g 10* ” when using amf encoder.
> > This parameter will set gop size to 10.
> >
> > вт, 11 апр. 2023 г. в 09:42, 罗勇刚(Yonggang Luo) :
> >
> > > When using libx264:
> > > The pull result is:
> > > ```
> > > C:\work\xtal\xtal-video\ffmpeg-6.0-full_build-shared\bin>ffplay
> > -max_delay
> > > 0 -max_probe_packets 1 -analyzeduration 0 -flags +low_delay -fflags
> > > +nobuffer -protocol_whitelist file,udp,rtp rtp_h264_30003.sdp
> > > ffplay version 6.0-full_build-www.gyan.dev Copyright (c) 2003-2023 the
> > > FFmpeg developers
> > >   built with gcc 12.2.0 (Rev10, Built by MSYS2 project)
> > >   configuration: --enable-gpl --enable-version3 --enable-shared
> > > --disable-w32threads --disable-autodetect --enable-fontconfig
> > > --enable-iconv --enable-gnutls --enable-libxml2 --enable-gmp
> > --enable-bzlib
> > > --enable-lzma --enable-libsnappy --enable-zlib --enable-librist
> > > --enable-libsrt --enable-libssh --enable-libzmq --enable-avisynth
> > > --enable-libbluray --enable-libcaca --enable-sdl2 --enable-libaribb24
> > > --enable-libdav1d --enable-libdavs2 --enable-libuavs3d --enable-libzvbi
> > > --enable-librav1e --enable-libsvtav1 --enable-libwebp --enable-libx264
> > > --enable-libx265 --enable-libxavs2 --enable-libxvid --enable-libaom
> > > --enable-libjxl --enable-libopenjpeg --enable-libvpx
> > > --enable-mediafoundation --enable-libass --enable-frei0r
> > > --enable-libfreetype --enable-libfribidi --enable-liblensfun
> > > --enable-libvidstab --enable-libvmaf --enable-libzimg --enable-amf
> > > --enable-cuda-llvm --enable-cuvid --enable-ffnvcodec --enable-nvdec
> > > --enable-nvenc --enable-d3d11va --enable-dxva2 --enable-libvpl
> > > --enable-libshaderc --enable-vulkan --enable-libplacebo --enable-opencl
> > > --enable-libcdio --enable-libgme --enable-libmodplug
> --enable-libopenmpt
> > > --enable-libopencore-amrwb --enable-libmp3lame --enable-libshine
> > > --enable-libtheora --enable-libtwolame --enable-libvo-amrwbenc
> > > --enable-libilbc --enable-libgsm --enable-libopencore-amrnb
> > > --enable-libopus --enable-libspeex --enable-libvorbis --enable-ladspa
> > > --enable-libbs2b --enable-libflite --enable-libmysofa
> > > --enable-librubberband --enable-libsoxr --enable-chromaprint
> > >   libavutil  58.  2.100 / 58.  2.100
> > >   libavcodec 60.  3.100 / 60.  3.100
> > >   libavformat60.  3.100 / 60.  3.100
> > >   libavdevice60.  1.100 / 60.  1.100
> > >   libavfilter 9.  3.100 /  9.  3.100
> > >   libswscale  7.  1.100 /  7.  1.100
> > >   libswresample   4. 10.100 /  4. 10.100
> > >   libpostproc57.  1.100 / 57.  1.100
> > > Input #0, sdp, from 'rtp_h264_30003.sdp':=0KB sq=0B f=0/0
> > >   Duration: N/A, start: -0.017911, bitrate: N/A
> > >   Stream #0:0: Video: h264 (High), yuv420p(progressive), 1920x1060, 50
> > fps,
> > > 50 tbr, 90k tbn
> > >3.79 M-V:  3.731 fd=   0 aq=0KB vq=0KB sq=0B f=0/0
> > > ```
> > >
> > > But when using h264_amf:
> > > ```
> > >
> > > C:\work\xtal\xtal-video\ffmpeg-6.0-full_build-shared\bin>ffplay
> > -max_delay
> > > 0 -max_probe_packets 1 -analyzeduration 0 -flags +low_delay -fflags
> > > +nobuffer -protocol_whitelist file,udp,rtp rtp_h264_30003.sdp
> > > ffplay version 6.0-full_build-www.gyan.dev Copyright (c) 2003-2023 the
> > > FFmpeg developers
> > >   built with gcc 12.2.0 (Rev10, Built by MSYS2 project)
> > >   configuration: --enable-gpl --enable-version3 --enable-shared
> > > --disable-w32threads --disable-autodetect --enable-fontconfig
> > > --enable-iconv --enable-gnutls --enable-libxml2 --enable-gmp
> > --enable-bzlib
> > > --enable-lzma --enable-libsnappy --enable-zlib --enable-librist
> > > --enable-libsrt --enable-libssh --enable-libzmq --enable-avisynth
> > > --enable-libbluray --enable-libcaca --enable-sdl2 --enable-libaribb24
> > > --enable-libdav1d --enable-libdavs2 --enable-libuavs3d --enable-libzvbi
> > > --enable-librav1e --enable-libsvtav1 --enable-libwebp --enable-libx264
> > > --enable-libx265 --enable-libxavs2 --enable-libxvid --enable-libaom
> > >