On 06/14/2011 02:08 AM, Anton Khirnov wrote:
> From: Nicolas George <[email protected]>
>
> Currently quad, 5.0, 5.1 and 7.1 are implemented.
> Implementing support for other formats/layouts and capture should be
> straightforward.
>
> 5.0 and 7.1 support by Carl Eugen Hoyos.
> +REORDER_OUT_50(int16, int16_t)
> +REORDER_OUT_51(int16, int16_t)
> +REORDER_OUT_71(int16, int16_t)
> +REORDER_OUT_50(int32, int32_t)
> +REORDER_OUT_51(int32, int32_t)
> +REORDER_OUT_71(int32, int32_t)
> +REORDER_OUT_50(f32, float)
> +REORDER_OUT_51(f32, float)
> +REORDER_OUT_71(f32, float)
what about an int8 version for S8, U8, MULAW, and ALAW?
> +
> +#define REORDER_DUMMY ((void *)1)
> +
> +#define FORMAT_I16 0
> +#define FORMAT_I32 1
> +#define FORMAT_F32 2
> +static av_cold ff_reorder_func find_reorder_func(int codec_id,
> + int64_t layout,
> + int out)
> +{
> + int format;
> +
> + /* reordering input is not currently supported */
> + if (!out)
> + return NULL;
> +
> + /* reordering is not needed for QUAD or 2_2 layout */
> + if (layout == AV_CH_LAYOUT_QUAD || layout == AV_CH_LAYOUT_2_2)
> + return REORDER_DUMMY;
> +
> + if ( codec_id == CODEC_ID_PCM_S16LE || codec_id ==
> CODEC_ID_PCM_S16BE ||
> + codec_id == CODEC_ID_PCM_U16LE || codec_id ==
> CODEC_ID_PCM_U16BE)
> + format = FORMAT_I16;
> + else if (codec_id == CODEC_ID_PCM_S32LE || codec_id ==
> CODEC_ID_PCM_S32BE ||
> + codec_id == CODEC_ID_PCM_U32LE || codec_id ==
> CODEC_ID_PCM_U32BE)
> + format = FORMAT_I32;
> + else if (codec_id == CODEC_ID_PCM_F32LE || codec_id ==
> CODEC_ID_PCM_F32BE)
> + format = FORMAT_F32;
> + else
> + return NULL;
I think this would be much more readable as a switch statement.
switch (codec_id) {
case CODEC_ID_PCM_S16LE:
case CODEC_ID_PCM_S16BE:
case CODEC_ID_PCM_U16LE:
case CODEC_ID_PCM_U16BE: format = FORMAT_I16; break;
case CODEC_ID_PCM_S32LE:
case CODEC_ID_PCM_S32BE:
case CODEC_ID_PCM_U32LE:
case CODEC_ID_PCM_U32BE: format = FORMAT_I32; break;
case CODEC_ID_PCM_F32LE:
case CODEC_ID_PCM_F32BE: format = FORMAT_F32; break;
default: return NULL;
}
> +
> + if (layout == AV_CH_LAYOUT_5POINT1_BACK || layout ==
> AV_CH_LAYOUT_5POINT1)
> + switch (format) {
> + case FORMAT_I16: return alsa_reorder_int16_out_51;
> + case FORMAT_I32: return alsa_reorder_int32_out_51;
> + case FORMAT_F32: return alsa_reorder_f32_out_51;
> + }
> +
> + if (layout == AV_CH_LAYOUT_5POINT0_BACK || layout ==
> AV_CH_LAYOUT_5POINT0)
> + switch (format) {
> + case FORMAT_I16: return alsa_reorder_int16_out_50;
> + case FORMAT_I32: return alsa_reorder_int32_out_50;
> + case FORMAT_F32: return alsa_reorder_f32_out_50;
> + }
> +
> + if (layout == AV_CH_LAYOUT_7POINT1)
> + switch (format) {
> + case FORMAT_I16: return alsa_reorder_int16_out_71;
> + case FORMAT_I32: return alsa_reorder_int32_out_71;
> + case FORMAT_F32: return alsa_reorder_f32_out_71;
> + }
> +
> + return NULL;
> +}
Why don't we just pass an AlsaData* and set the function pointer
directly? That avoids having to typedef the function and doesn't need
REORDER_DUMMY since it could return an error code.
> + char name[32];
> + av_get_channel_layout_string(name, sizeof(name), channels,
> layout);
This needs to be more than 32 characters. e.g. 8-channel layout names
without a pre-defined name will likely barely fit, and any more than
that probably won't fit.
> +
> +int ff_alsa_extend_reorder_buf(AlsaData *s, int min_size)
> +{
> + int size = s->reorder_buf_size;
> + void *r;
> +
> + while (size < min_size)
> + size *= 2;
assert(size != 0) would be nice to have before the potential infinite loop.
-Justin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
