mbox series

[FFmpeg-devel,v3,0/5] Support for stream dispositions in MP4

Message ID 20210920150048.8790-1-jeebjp@gmail.com
Headers show
Series Support for stream dispositions in MP4
Related show

Message

Jan Ekström Sept. 20, 2021, 3 p.m. UTC
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.

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

Best regards,
Jan

Jan Ekström (5):
  avformat/aviobuf: add a full string reading mode to
    read_line_to_bprint
  avformat/{aviobuf,avio_internal}: add
    ff_read_string_to_bprint_overwrite
  avformat/{aviobuf,avio_internal}: add max_len argument to
    ff_read_string_to_bprint_overwrite
  avformat/{isom,mov,movenc}: add support for CMAF DASH roles
  avformat/{isom,movenc}: add kind box compatibility mode for Unified
    Origin

 libavformat/avio_internal.h                   | 19 ++++
 libavformat/aviobuf.c                         | 40 ++++++--
 libavformat/isom.c                            | 32 +++++++
 libavformat/isom.h                            | 19 ++++
 libavformat/mov.c                             | 91 +++++++++++++++++++
 libavformat/movenc.c                          | 57 ++++++++++++
 libavformat/movenc.h                          |  2 +
 tests/fate/mov.mak                            | 17 ++++
 .../ref/fate/mov-mp4-disposition-mpegts-remux | 81 +++++++++++++++++
 ...p4-disposition-unified-origin-mpegts-remux | 81 +++++++++++++++++
 10 files changed, 432 insertions(+), 7 deletions(-)
 create mode 100644 tests/ref/fate/mov-mp4-disposition-mpegts-remux
 create mode 100644 tests/ref/fate/mov-mp4-disposition-unified-origin-mpegts-remux

Comments

Jan Ekström Sept. 27, 2021, 10:53 a.m. UTC | #1
On Mon, Sep 20, 2021 at 6:00 PM Jan Ekström <jeebjp@gmail.com> 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
Jan Ekström Oct. 4, 2021, 3:36 p.m. UTC | #2
On Mon, Sep 27, 2021 at 1:53 PM Jan Ekström <jeebjp@gmail.com> wrote:
>
> On Mon, Sep 20, 2021 at 6:00 PM Jan Ekström <jeebjp@gmail.com> 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