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

Reply via email to