On Wed, 24 Sep 2014, Omer Osman wrote:
Am 24.09.2014 17:07, schrieb Justin Ruggles:
On 09/24/2014 10:48 AM, Omer Osman wrote:
+ switch (downmix_channels){
+ case 2:
+ case 1:
+ if(aacDecoder_SetParam(s->handle, AAC_PCM_OUTPUT_CHANNELS,
downmix_channels) != AAC_DEC_OK){
+ av_log(avctx, AV_LOG_ERROR, "Unable to set output
channels in the decoder\n");
+ return AVERROR_UNKNOWN;
+ }
+ break;
If FDK-AAC actually downmixes to mono and stereo, those channel layouts
should be checked, not the channel count.
int downmix_channels =
av_get_channel_layout_nb_channels(avctx->request_channel_layout); <<
Therefore, the number of channels from the channel layout is being checked.
+ default:
+ av_log(avctx, AV_LOG_ERROR, "Invalid
request_channel_layout\n");
+ return AVERROR_UNKNOWN;
+ }
It's actually fine for the decoder to ignore the requested layout and not
fail.
OK by me, however, I think it makes sense to at least throw a warning.
+ frame->nb_samples = avctx->frame_size;
+ if ((ret = ff_get_buffer(avctx, frame, 0)) < 0)
+ goto end;
+ memcpy(frame->extended_data[0], s->decoder_buffer,
+ avctx->channels * avctx->frame_size *
+ av_get_bytes_per_sample(avctx->sample_fmt));
I'm not thrilled about making the memcpy() unconditional. Can this be
avoided when not downmixing?
No, this is required in all cases, and was part of the original code.
(tmpptr/buf was being used after initialization)
I'm not quite sure I understand - previously, we decoded straight into the
frame buffer once we knew the number of channels (where buf =
frame->extended_data[0], tmpptr = NULL), while it after this patch always
will decode into an intermediate buffer. It would be desireable to keep
the old behaviour with fewer copies at least as long as no specific
channel layout has been requested.
// Martin
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel