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 770678 - qtdemux: Not considering sample size when calculating size of chunk.
qtdemux: Not considering sample size when calculating size of chunk.
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-08-31 20:58 UTC by wesfrisby1
Modified: 2018-11-03 15:11 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
stsz sample size is 1 (67.06 KB, image/png)
2017-01-25 23:30 UTC, wesfrisby1
  Details
Does not work when sample size is 640 (76.34 KB, image/png)
2017-01-25 23:30 UTC, wesfrisby1
  Details
Patch to fix issues when stream->sample_size is not one. (1000 bytes, patch)
2017-01-25 23:40 UTC, wesfrisby1
reviewed Details | Review

Description wesfrisby1 2016-08-31 20:58:15 UTC
The size of chunk is not taking into account the size of samples. Some mp4 files have stream->sample_size != 1. When this occurs they will not play audio. If stream->sample_size is 1 the current code works. 

What I'm proposing is a change here to lines: 7799/7800

         if (stream->samples_per_frame * stream->bytes_per_frame) {
           cur->size =
-              (stream->samples_per_chunk * stream->n_channels) /
-              stream->samples_per_frame * stream->bytes_per_frame;
+              ((stream->samples_per_chunk * stream->n_channels) /
+              stream->samples_per_frame) * stream->bytes_per_frame * stream->sample_size;
         } else {
           cur->size = stream->samples_per_chunk;
         }
Comment 1 Tim-Philipp Müller 2016-09-04 17:27:55 UTC
Could you make such a file available for us by any chance? (If it's <1MB you can also attach it here.)
Comment 2 wesfrisby1 2016-09-06 17:53:46 UTC
I don't have a small clip available unfortunately. Here is a dropbox link to an example clip: https://www.dropbox.com/s/bo35zh41abqowmm/CISCO.zip?dl=0
Comment 3 wesfrisby1 2016-09-15 22:23:41 UTC
Has anyone else been able to verify this should fix the issue and introduce other bugs?

Thanks,
Wes
Comment 4 wesfrisby1 2017-01-25 23:30:03 UTC
Created attachment 344278 [details]
stsz sample size is 1
Comment 5 wesfrisby1 2017-01-25 23:30:55 UTC
Created attachment 344279 [details]
Does not work when sample size is 640
Comment 6 wesfrisby1 2017-01-25 23:40:47 UTC
Created attachment 344280 [details] [review]
Patch to fix issues when stream->sample_size is not one.

Patch for qtdemux on version 1.10.2 lets you play sections that do not have sample size of 1.
Comment 7 Sebastian Dröge (slomo) 2017-01-26 12:56:31 UTC
Review of attachment 344280 [details] [review]:

::: gst/isomp4/qtdemux.c
@@ +8299,3 @@
           cur->size =
               (stream->samples_per_chunk * stream->n_channels) /
+              stream->samples_per_frame * stream->bytes_per_frame * stream->sample_size;

This looks correct (but might need some care to prevent overflows?)

@@ +8314,3 @@
         cur++;
 
+        stream->stco_sample_index += stream->samples_per_chunk * stream->sample_size;

But this seems not obviously correct at least. Why would the sample_index take the sample_size into account? Shouldn't that just take the number of samples?
Comment 8 GStreamer system administrator 2018-11-03 15:11:18 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/295.