[FFmpeg-devel] [PATCH 2/2] spdifenc: fix byte order on big-endian systems

[Janne Grunau]

On Fri, Jan 21, 2011 at 08:33:00PM +0200, Anssi Hannula wrote:
> There is a check for HAVE_BIGENDIAN when outputting the IEC 61937
> stream. On big-endian systems the payload data is not byteswapped,
> causing in effect the outputted payload data to be in a different byte
> order on big-endian than on little-endian systems.
> 
> However, the IEC 61937 preamble (and the final odd byte if present) is
> always outputted in the same byte order. This means that on big-endian
> systems the headers have a different byte order than the payload,
> preventing useful use of the output.
> 
> Fix that by outputting the data in a format suitable for sending to an
> audio device in S16LE format by default. Output as big-endian (S16BE)
> is added as an AVOption. This makes the muxer output the same on all
> archs by default.
> 
> ---
> 
> Other ways to fix this would be to
> a) simply always output in little-endian format, or
> b) always output in native-endian format (i.e. different muxer output
> depending on arch), or
> c) have two different logical muxers.

I think I prefer this solution.

> 
>  libavformat/spdifenc.c |   26 ++++++++++++++++++++------
>  1 files changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/libavformat/spdifenc.c b/libavformat/spdifenc.c
> index 8e35190..f85be8a 100644
> --- a/libavformat/spdifenc.c
> +++ b/libavformat/spdifenc.c
> @@ -76,6 +76,8 @@ typedef struct IEC61937Context {
>      /* AVOptions: */
>      int dtshd_rate;
>      int dtshd_fallback;
> +#define SPDIF_FLAG_BIGENDIAN    0x01
> +    int spdif_flags;
>  
>      /// function, which generates codec dependent header information.
>      /// Sets data_type and pkt_offset, and length_code, out_bytes, out_buf if necessary
> @@ -83,6 +85,8 @@ typedef struct IEC61937Context {
>  } IEC61937Context;
>  
>  static const AVOption options[] = {
> +{ "spdif_flags", "IEC 61937 encapsulation flags", offsetof(IEC61937Context, spdif_flags), FF_OPT_TYPE_FLAGS, 0, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "spdif_flags" },
> +{ "be", "output in big-endian format (for use as s16be)", 0, FF_OPT_TYPE_CONST, SPDIF_FLAG_BIGENDIAN, 0, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM, "spdif_flags" },
>  { "dtshd_rate", "mux complete DTS frames in HD mode at the specified IEC958 rate (in Hz, default 0=disabled)", offsetof(IEC61937Context, dtshd_rate), FF_OPT_TYPE_INT, 0, 0, 768000, AV_OPT_FLAG_ENCODING_PARAM },
>  { "dtshd_fallback_time", "min secs to strip HD for after an overflow (-1: till the end, default 60)", offsetof(IEC61937Context, dtshd_fallback), FF_OPT_TYPE_INT, 60, -1, INT_MAX, AV_OPT_FLAG_ENCODING_PARAM },
>  { NULL },
> @@ -476,6 +480,15 @@ static int spdif_write_trailer(AVFormatContext *s)
>      return 0;
>  }
>  
> +static void spdif_put_16(struct AVFormatContext *s, unsigned int val)
> +{

av_always_inline

> +    IEC61937Context *ctx = s->priv_data;

just pass the IEC61937Context pointer as argument

> +    if (ctx->spdif_flags & SPDIF_FLAG_BIGENDIAN)
> +        put_be16(s->pb, val);
> +    else
> +        put_le16(s->pb, val);
> +}

I'm not sure if this is the cleanest solution. factoring the data writing
parts into spdif_write_packet_data_le|be might be nicer. Has anyone else
an opinion?

> +
>  static int spdif_write_packet(struct AVFormatContext *s, AVPacket *pkt)
>  {
>      IEC61937Context *ctx = s->priv_data;
> @@ -500,13 +513,13 @@ static int spdif_write_packet(struct AVFormatContext *s, AVPacket *pkt)
>      }
>  
>      if (ctx->use_preamble) {
> -        put_le16(s->pb, SYNCWORD1);       //Pa
> -        put_le16(s->pb, SYNCWORD2);       //Pb
> -        put_le16(s->pb, ctx->data_type);  //Pc
> -        put_le16(s->pb, ctx->length_code);//Pd
> +        spdif_put_16(s, SYNCWORD1);       //Pa
> +        spdif_put_16(s, SYNCWORD2);       //Pb
> +        spdif_put_16(s, ctx->data_type);  //Pc
> +        spdif_put_16(s, ctx->length_code);//Pd
>      }
>  
> -    if (HAVE_BIGENDIAN ^ ctx->extra_bswap) {
> +    if (ctx->extra_bswap ^ (ctx->spdif_flags & SPDIF_FLAG_BIGENDIAN)) {
>      put_buffer(s->pb, ctx->out_buf, ctx->out_bytes & ~1);
>      } else {
>      av_fast_malloc(&ctx->buffer, &ctx->buffer_size, ctx->out_bytes + FF_INPUT_BUFFER_PADDING_SIZE);
> @@ -516,8 +529,9 @@ static int spdif_write_packet(struct AVFormatContext *s, AVPacket *pkt)
>      put_buffer(s->pb, ctx->buffer, ctx->out_bytes & ~1);
>      }
>  
> +    /* a final lone byte has to be MSB aligned */
>      if (ctx->out_bytes & 1)
> -        put_be16(s->pb, ctx->out_buf[ctx->out_bytes - 1]);
> +        spdif_put_16(s, ctx->out_buf[ctx->out_bytes - 1] << 8);
>  
>      put_nbyte(s->pb, 0, padding);
>  

otherwise ok

Janne