[FFmpeg-devel] [PATCH] avformat/mxfenc: Allow overriding /manual setting of the signal standard

[Michael Niedermayer]

On Mon, Jun 08, 2015 at 09:48:33AM +0100, tim nicholson wrote:
> On 06/06/15 02:23, Michael Niedermayer wrote:
> > Signed-off-by: Michael Niedermayer <michaelni at gmx.at>
> > ---
> >  libavformat/mxfenc.c |   10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/libavformat/mxfenc.c b/libavformat/mxfenc.c
> > index c612747..f2a7f0a 100644
> > --- a/libavformat/mxfenc.c
> > +++ b/libavformat/mxfenc.c
> > @@ -316,6 +316,7 @@ typedef struct MXFContext {
> >      uint32_t instance_number;
> >      uint8_t umid[16];        ///< unique material identifier
> >      int channel_count;
> > +    int signal_standard;
> >      uint32_t tagged_value_count;
> >      AVRational audio_edit_rate;
> >  } MXFContext;
> > @@ -2104,6 +2105,8 @@ static int mxf_write_header(AVFormatContext *s)
> >  
> >                  sc->signal_standard = 1;
> >              }
> > +            if (mxf->signal_standard >= 0)
> > +                sc->signal_standard = mxf->signal_standard;
> >          } else if (st->codec->codec_type == AVMEDIA_TYPE_AUDIO) {
> >              if (st->codec->sample_rate != 48000) {
> >                  av_log(s, AV_LOG_ERROR, "only 48khz is implemented\n");
> > @@ -2627,7 +2630,12 @@ static int mxf_interleave(AVFormatContext *s, AVPacket *out, AVPacket *pkt, int
> >                                 mxf_interleave_get_packet, mxf_compare_timestamps);
> >  }
> >  
> > +#define COMMON_OPTIONS \
> 
> nit MXF_COMMON_OPTIONS ?
> feels more comfortable to me when grepping the code etc.
> 
> > +    { "signal_standard", "Force/set Sigal Standard",\
> > +      offsetof(MXFContext, signal_standard), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 8, AV_OPT_FLAG_ENCODING_PARAM},
> > +
> 
> Isn't 07h the largest valid value (G.2.3)? Or do I misunderstand the
> constraint values? (AVOPTION not my strong point)
> 
> I thought numeric options were deprecated these days in favour of more
> understandable string values, or are the string values also too obscure
> to be useful in this case? (Just wondering what the current protocol was
> on this nowadays, not a real concern in this case)

changed


> 
> >  static const AVOption mxf_options[] = {
> > +    COMMON_OPTIONS
> >      { NULL },
> >  };
> >  
> > @@ -2641,6 +2649,7 @@ static const AVClass mxf_muxer_class = {
> >  static const AVOption d10_options[] = {
> >      { "d10_channelcount", "Force/set channelcount in generic sound essence descriptor",
> >        offsetof(MXFContext, channel_count), AV_OPT_TYPE_INT, {.i64 = -1}, -1, 8, AV_OPT_FLAG_ENCODING_PARAM},
> > +    COMMON_OPTIONS
> >      { NULL },
> >  };
> >  
> > @@ -2654,6 +2663,7 @@ static const AVClass mxf_d10_muxer_class = {
> >  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 },
> > +    COMMON_OPTIONS
> >      { NULL },
> >  };
> >  
> > 
> 
> otherwise LGTM.

applied

thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20150608/051099ec/attachment.asc>