[FFmpeg-devel] [PATCH 2/2] spdifenc: fix byte order on big-endian systems
Anssi Hannula
anssi.hannula
Fri Feb 11 04:24:02 CET 2011
On 10.02.2011 23:46, Janne Grunau wrote:
> 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.
OK. I slightly prefer the AVOption to avoid small things like "ffmpeg
-help" listing the same AVOptions twice for both logical muxers.
However, I don't have a strong opinion on this so I can make it two
logical muxers, then (unless other people chime in).
>>
>> 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
OK.
>> + IEC61937Context *ctx = s->priv_data;
>
> just pass the IEC61937Context pointer as argument
OK, though then I'll have to pass ByteIOContext* as well.
>> + 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?
Wouldn't it cause some unnecessary code duplication?
E.g.:
int spdif_write_packet_data_le()
{
if (ctx->use_preamble) {
put_le16(s, SYNCWORD1); //Pa
put_le16(s, SYNCWORD2); //Pb
put_le16(s, ctx->data_type); //Pc
put_le16(s, ctx->length_code);//Pd
}
if (ctx->extra_bswap) {
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);
if (!ctx->buffer)
return AVERROR(ENOMEM);
ff_spdif_bswap_buf16((uint16_t *)ctx->buffer, (uint16_t
*)ctx->out_buf, ctx->out_bytes >> 1);
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, ctx->out_buf[ctx->out_bytes - 1]);
}
int spdif_write_packet_data_be()
{
if (ctx->use_preamble) {
put_be16(s, SYNCWORD1); //Pa
put_be16(s, SYNCWORD2); //Pb
put_be16(s, ctx->data_type); //Pc
put_be16(s, ctx->length_code);//Pd
}
if (!ctx->extra_bswap) {
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);
if (!ctx->buffer)
return AVERROR(ENOMEM);
ff_spdif_bswap_buf16((uint16_t *)ctx->buffer, (uint16_t
*)ctx->out_buf, ctx->out_bytes >> 1);
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_le16(s, ctx->out_buf[ctx->out_bytes - 1]);
}
>> +
>> 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
--
Anssi Hannula
More information about the ffmpeg-devel
mailing list