Review of attachment 291882 [details] [review]:

Thanks for the patch, generally looks good... just some comments below :)

::: omx/gstomxac3dec.c
@@ +66,3 @@
+       "channels=(int)[1,6], "
+       "rate=(int)[8000,48000], "
+       "alignment=(string){iec61937,frame}";

Are you sure both are always supported?

@@ +90,3 @@
+  GstOMXAC3Dec *self = GST_OMX_AC3_DEC (dec);
+  OMX_PARAM_PORTDEFINITIONTYPE port_def;
+  OMX_AUDIO_PARAM_AACPROFILETYPE ac3_param;

AAC

@@ +110,3 @@
+
+  err =
+      gst_omx_component_get_parameter (dec->dec, OMX_IndexParamAudioAac,

AAC

@@ +121,3 @@
+  s = gst_caps_get_structure (caps, 0);
+
+  if (/*!gst_structure_get_int (s, "mpegversion", &mpegversion) ||*/

No mpegversion field

@@ +148,3 @@
+{
+  GstOMXAC3Dec *self = GST_OMX_AC3_DEC (dec);
+  OMX_AUDIO_PARAM_AACPROFILETYPE ac3_param;

AAC

@@ +158,3 @@
+
+  err =
+      gst_omx_component_get_parameter (dec->dec, OMX_IndexParamAudioAac,

Should be somethnig with Ac3 instead of AAC

@@ +169,3 @@
+  s = gst_caps_get_structure (caps, 0);
+
+  if (/*!gst_structure_get_int (s, "mpegversion", &mpegversion) ||*/

There is no mpegversion field for AC3

@@ +209,3 @@
+  }
+
+  /* FIXME: Rather arbitrary values here, based on what we do in gstfaac.c */

IIRC the channel positions are fixed in AC3, so we can choose proper values here

::: omx/gstomxac3dec.h
@@ +45,3 @@
+{
+  GstOMXAudioDec parent;
+  gint spf;

No need to have that here as AC3 always has 512 samples per frame (IIRC)

::: omx/gstomxaudiodec.c
@@ +531,3 @@
     nframes = 1;
+    if (self->parent.element.object.name == "omxac3dec-omxac3dec0")
+       spf = 512;

You would implement the get_samples_per_frame() vfunc in gstomxac3dec and return 512 there instead. Like it's done for AAC and MP3 already.

(In reply to kalyan from comment #0)
> i  implemented AC3 audio and tested

Was it on desktop or an embedded device ? Which omx target ? Strange that it worked using OMX_IndexParamAudioAac.

Note that ac3 is not in the headers provided by Khronos.
For Android and it comes as an extension: https://android.googlesource.com/platform/frameworks/native/+/master/include/media/openmax/OMX_IndexExt.h#60 .
For Rpi/Broadcom it comes like that: https://github.com/raspberrypi/firmware/blob/master/opt/vc/include/IL/OMX_Index.h#L392

Closing this bug report as no further information has been provided. Please feel free to reopen this bug report if you can provide the information that was asked for in a previous comment.
Thanks!