On Tue, 29 Aug 2017 15:02:49 +0200 Michael Niedermayer <mich...@niedermayer.cc> wrote:
> On Tue, Aug 29, 2017 at 12:02:18PM +0200, wm4 wrote: > > On Tue, 29 Aug 2017 11:59:05 +0200 > > Michael Niedermayer <mich...@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 <mich...@niedermayer.cc> wrote: > > > > > > > > > Signed-off-by: Michael Niedermayer <mich...@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. > > It should be possible to generate mxf files with just one tool not 2. > In fact this change is for someone else not me, i probably wont be > using it much. MXF is such a complex format that FFmpeg can't hope it can generate all possible MXF variants and use all of its possible features. There are plenty of mp4/mkv/other features that FFmpeg can't write. So that's no justification on its own. I could probably send 100s of patches adding tiny unneeded features for writing obscure mkv elements using private options. Sometimes, such features have a real justification because either a feature doesn't fit into FFmpeg's abstractions, or because it's needed as a workaround to deal with multiple broken MXF readers, but I've not seen that from you either. > Do you object to this patch ? > If you veto it, i will not apply it of course. Yes. > > if not i will apply it with the lines reordered to avoid the \ issue > raised by paul > as FFmpeg is intended to be a universal multimedia tool and requiring > a 2nd tool is cumbersome. Oh, that's a great reason to dump unneeded crap into FFmpeg. _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel