[FFmpeg-devel] avformat/mxfenc: allow user comments for opatom muxer

Submitted by Mark Reid on March 11, 2019, 2:03 a.m.

Details

Message ID 20190311020307.6680-1-mindmark@gmail.com
State New
Headers show

Commit Message

Mark Reid March 11, 2019, 2:03 a.m.
From: Mark Reid <mindmark@gmail.com>

This patch restores the ability to add user comments for the opatom_mxf muxer.
The ability seems to have been disabled in d9726893f31.

---
 doc/muxers.texi      | 2 +-
 libavformat/mxfenc.c | 2 ++
 2 files changed, 3 insertions(+), 1 deletion(-)

--
2.18.0

Comments

Tomas Härdin March 11, 2019, 9:55 a.m.
sön 2019-03-10 klockan 19:03 -0700 skrev mindmark@gmail.com:
> > From: Mark Reid <mindmark@gmail.com>
> 
> This patch restores the ability to add user comments for the opatom_mxf muxer.
> The ability seems to have been disabled in d9726893f31.

Seems the intent was to only disable them for D-10, so this is probably
fine. You should change this too:

  IRT D-10 does not allow user comments. The default is thus to write
  them for mxf but not for mxf_d10

to

  IRT D-10 does not allow user comments. The default is thus to write
  them for mxf and mxf_opatom but not for mxf_d10

The comment from Michael in d9726893f31 about comments causing problems
for "users" has me worried. We *need* functional tests for stuff like
that..

/Tomas
Mark Reid March 11, 2019, 8:15 p.m.
On Mon, Mar 11, 2019 at 2:55 AM Tomas Härdin <tjoppen@acc.umu.se> wrote:

> sön 2019-03-10 klockan 19:03 -0700 skrev mindmark@gmail.com:
> > > From: Mark Reid <mindmark@gmail.com>
> >
> > This patch restores the ability to add user comments for the opatom_mxf
> muxer.
> > The ability seems to have been disabled in d9726893f31.
>
> Seems the intent was to only disable them for D-10, so this is probably
> fine. You should change this too:
>
>   IRT D-10 does not allow user comments. The default is thus to write
>   them for mxf but not for mxf_d10
>
> to
>
>   IRT D-10 does not allow user comments. The default is thus to write
>   them for mxf and mxf_opatom but not for mxf_d10
>
> oops missed the line.


> The comment from Michael in d9726893f31 about comments causing problems
> for "users" has me worried. We *need* functional tests for stuff like
> that..
>
>
I'll try and add a fate test with the next version of the patch. Thanks for
the review.


> /Tomas
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Patch hide | download patch | download mbox

diff --git a/doc/muxers.texi b/doc/muxers.texi
index 372fab2f92..764102bf4b 100644
--- a/doc/muxers.texi
+++ b/doc/muxers.texi
@@ -1629,7 +1629,7 @@  ffmpeg -i file.mpg -c copy \
      out.ts
 @end example

-@section mxf, mxf_d10
+@section mxf, mxf_d10, mxf_opatom

 MXF muxer.

diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
index 032ee3bf3d..8c6db94865 100644
--- a/libavformat/mxfenc.c
+++ b/libavformat/mxfenc.c
@@ -3095,6 +3095,8 @@  static const AVOption opatom_options[] = {
     { "mxf_audio_edit_rate", "Audio edit rate for timecode",
         offsetof(MXFContext, audio_edit_rate), AV_OPT_TYPE_RATIONAL, {.dbl=25}, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM },
     MXF_COMMON_OPTIONS
+    { "store_user_comments", "",
+      offsetof(MXFContext, store_user_comments), AV_OPT_TYPE_BOOL, {.i64 = 1}, 0, 1, AV_OPT_FLAG_ENCODING_PARAM},
     { NULL },
 };