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

[tim nicholson]

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)

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

-- 
Tim.
Key Fingerprint 38CF DB09 3ED0 F607 8B67 6CED 0C0B FC44 8B0B FC83