After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 784552 - dvdlpcmdec: Added support for Blu-Ray audio
dvdlpcmdec: Added support for Blu-Ray audio
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
unspecified
Other All
: Normal enhancement
: 1.13.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-07-05 15:59 UTC by Vivia Nikolaidou
Modified: 2017-07-05 16:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
dvdlpcmdec: Added support for Blu-Ray audio (10.14 KB, patch)
2017-07-05 15:59 UTC, Vivia Nikolaidou
none Details | Review
dvdlpcmdec: Added support for Blu-Ray audio (9.98 KB, patch)
2017-07-05 16:11 UTC, Vivia Nikolaidou
committed Details | Review

Description Vivia Nikolaidou 2017-07-05 15:59:38 UTC
See commit message
Comment 1 Vivia Nikolaidou 2017-07-05 15:59:42 UTC
Created attachment 354935 [details] [review]
dvdlpcmdec: Added support for Blu-Ray audio
Comment 2 Sebastian Dröge (slomo) 2017-07-05 16:09:23 UTC
Review of attachment 354935 [details] [review]:

::: gst/dvdlpcmdec/gstdvdlpcmdec.c
@@ +348,3 @@
 
+  gst_dvdlpcmdec_update_audio_formats (dvdlpcmdec, channels, rate, format,
+      channels, channel_positions);

channels-1

@@ +420,3 @@
   channels = ((header >> 8) & 0x7) + 1;
 
+  gst_dvdlpcmdec_update_audio_formats (dec, channels, rate, format, channels,

channels-1

@@ +610,3 @@
+      case 0x2:
+        format = GST_AUDIO_FORMAT_S32BE;
+        dvdlpcmdec->width = 20;

This is either also GST_AUDIO_FORMAT_S24BE or GST_AUDIO_FORMAT_S20BE. The code in ffmpeg suggests it's the former though

@@ +614,3 @@
+      case 0x3:
+        format = GST_AUDIO_FORMAT_S32BE;
+        dvdlpcmdec->width = 24;

GST_AUDIO_FORMAT_S24BE AFAIU?

@@ +671,3 @@
+        break;
+    }
+    GST_DEBUG_OBJECT (dvdlpcmdec, "got channels %i rate %i format %s\n",

Usually we use %d for integers (decimal, vs %x for hexadecimal). And the \n should go away
Comment 3 Vivia Nikolaidou 2017-07-05 16:11:30 UTC
Created attachment 354939 [details] [review]
dvdlpcmdec: Added support for Blu-Ray audio
Comment 4 Sebastian Dröge (slomo) 2017-07-05 16:18:24 UTC
Attachment 354939 [details] pushed as cd59f39 - dvdlpcmdec: Added support for Blu-Ray audio