[FFmpeg-devel] [PATCH 1/3] avformat/mxfenc: Allow overriding numerical color_siting value.

[wm4]

On Tue, 29 Aug 2017 11:59:05 +0200
Michael Niedermayer <michael at niedermayer.cc> wrote:

> On Tue, Aug 29, 2017 at 11:02:32AM +0200, wm4 wrote:
> > On Tue, 29 Aug 2017 02:13:19 +0200
> > Michael Niedermayer <michael at niedermayer.cc> wrote:
> >   
> > > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > > ---
> > >  libavformat/mxfenc.c | 7 ++++++-
> > >  1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> > > index 12fc9abbc6..ccfa0d6341 100644
> > > --- a/libavformat/mxfenc.c
> > > +++ b/libavformat/mxfenc.c
> > > @@ -322,6 +322,7 @@ typedef struct MXFContext {
> > >      uint8_t umid[16];        ///< unique material identifier
> > >      int channel_count;
> > >      int signal_standard;
> > > +    int color_siting;
> > >      uint32_t tagged_value_count;
> > >      AVRational audio_edit_rate;
> > >      int store_user_comments;
> > > @@ -2085,6 +2086,8 @@ static int mxf_write_header(AVFormatContext *s)
> > >              case AVCHROMA_LOC_TOP:     sc->color_siting = 1; break;
> > >              case AVCHROMA_LOC_CENTER:  sc->color_siting = 3; break;
> > >              }
> > > +            if (mxf->color_siting >= 0)
> > > +                sc->color_siting = mxf->color_siting;
> > >  
> > >              mxf->timecode_base = (tbc.den + tbc.num/2) / tbc.num;
> > >              spf = ff_mxf_get_samples_per_frame(s, tbc);
> > > @@ -2668,7 +2671,9 @@ static int mxf_interleave(AVFormatContext *s, AVPacket *out, AVPacket *pkt, int
> > >      { "smpte349m", "SMPTE 349M (1485 Mbps mappings)",\
> > >        0, AV_OPT_TYPE_CONST, {.i64 = 6}, -1, 7, AV_OPT_FLAG_ENCODING_PARAM, "signal_standard"},\
> > >      { "smpte428", "SMPTE 428-1 DCDM",\
> > > -      0, AV_OPT_TYPE_CONST, {.i64 = 7}, -1, 7, AV_OPT_FLAG_ENCODING_PARAM, "signal_standard"},
> > > +      0, AV_OPT_TYPE_CONST, {.i64 = 7}, -1, 7, AV_OPT_FLAG_ENCODING_PARAM, "signal_standard"},\
> > > +    { "color_siting", "Force/set Color siting",\
> > > +      offsetof(MXFContext, color_siting), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 7, AV_OPT_FLAG_ENCODING_PARAM, "color_siting"},\
> > >  
> > >  
> > >    
> > 
> > What an absurd patch. This should be done by generic code overriding
> > the chroma_location field on AVFrames sent to the encoders. We don't
> > need inconsistent private options for an inconsistent set of encoders
> > to override generic parameters in an inconsistent way. What's even the
> > point?  
> 
> The point of this change is to be able to set any color siting value.
> MXF has some redundant values. The fields from AVFrame are not enough
> to choose this unless we want MXF specific information in AVFrames
> 
> [...]

You probably want to use some specialized MXF tool then.