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 710609 - asfdemux: only check mo_size if it is provided
asfdemux: only check mo_size if it is provided
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
1.0.7
Other All
: Normal normal
: 1.2.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2013-10-22 07:53 UTC by HyeJin Choi
Modified: 2013-11-11 18:55 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
bug fix - check mo_size field is not mandatory. So, don't need to mo_size value check when merge the payload. (1.16 KB, patch)
2013-10-22 07:53 UTC, HyeJin Choi
none Details | Review
Sample file to test mo_size (370.17 KB, video/x-ms-asf)
2013-10-28 04:44 UTC, HyeJin Choi
  Details
How about this patch? It is an improvement over yours as still does the same (1.46 KB, patch)
2013-10-28 13:14 UTC, Thiago Sousa Santos
reviewed Details | Review
new patch for fix mo_size bug (1.93 KB, patch)
2013-10-29 04:39 UTC, HyeJin Choi
committed Details | Review

Description HyeJin Choi 2013-10-22 07:53:23 UTC
Created attachment 257818 [details] [review]
bug fix - check mo_size field is not mandatory. So, don't need to mo_size value check when merge the payload.

Media object size is not mandatory field in ASF specification.

And actually, some file has no media object size field in payload except first payload of each ES data.
Like this....
1st payload : mo_id = 1, mo_offset = 0, mo_size = 8021 bytes
2nd payload : mo_id = 1, mo_offset = 2500  (has no mo_size)
3rd payload : mo_id = 1, mo_offset = 5000 (has no mo_size)

But asf demux always check the previous payload's media object size and current payload's media object size when merge it.

Cause of it, some file always fail to parse payload.
Comment 1 Thiago Sousa Santos 2013-10-23 11:54:09 UTC
Do you have an example asf file to reproduce this issue?
Comment 2 HyeJin Choi 2013-10-28 04:43:08 UTC
Sorry, I read your comment on Today.
I upload a sample file. You can reproduce with this.

Thanks.
Comment 3 HyeJin Choi 2013-10-28 04:44:52 UTC
Created attachment 258266 [details]
Sample file to test mo_size
Comment 4 Thiago Sousa Santos 2013-10-28 13:14:25 UTC
Created attachment 258300 [details] [review]
How about this patch? It is an improvement over yours as still does the same

check, but only then a mo_size is found.

What do you think?
Comment 5 Tim-Philipp Müller 2013-10-28 14:05:04 UTC
Comment on attachment 258300 [details] [review]
How about this patch? It is an improvement over yours as still does the same

>+          if (prev->buf == NULL || (payload.mo_size
>+                  && payload.mo_size != prev->mo_size)

Please use payload.mo_size > 0 or payload.mo_size != 0 here, since it's not a boolean.
Comment 6 HyeJin Choi 2013-10-29 04:39:37 UTC
Created attachment 258405 [details] [review]
new patch for fix mo_size bug

I attach a new patch that is apply your reviews.
And I suggest that initialize the mo_size variable for sure.
Please review this patch.
Comment 7 Thiago Sousa Santos 2013-10-29 15:12:32 UTC
Thanks! Pushed as 1e74f611ee8d1c6d65d0e7963ef5dc276a07e092.