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 607698 - asfdemux: fix parsing of packets with padding
asfdemux: fix parsing of packets with padding
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
git master
Other Linux
: Normal normal
: 0.10.18
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2010-01-21 18:34 UTC by Thiago Sousa Santos
Modified: 2010-01-22 19:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
the patch (1.66 KB, patch)
2010-01-21 18:35 UTC, Thiago Sousa Santos
none Details | Review

Description Thiago Sousa Santos 2010-01-21 18:34:50 UTC
+++ This bug was initially created as a clone of Bug #607555 +++

On this comment in 607555 a possible bug was found in asfdemux, so here is a clone to track the separate issue.

> Review of attachment 151838 [details] [review] [details]:
> 
> 1549      /* Due to a limitation in WMP while streaming through WMSP we reduce
> the
> 1520      GST_WRITE_UINT32_LE (data + 6, size_left);    /* padding size */     
>   1550       * packet & padding size to 16bit if theay are <= 65535 bytes 
> 1551       */
> 1552      if (asfmux->packet_size > 65535) {
> 1553        GST_WRITE_UINT32_LE (data + offset, asfmux->packet_size -
> size_left);
> 1554        offset += 4;
> 1555      } else {
> 1556        *data &= ~(ASF_FIELD_TYPE_MASK << 5);
> 1557        *data |= ASF_FIELD_TYPE_WORD << 5;
> 1558        GST_WRITE_UINT16_LE (data + offset, asfmux->packet_size -
> size_left);
> 1559        offset += 2;
> 1560      }
> 
> Why are you using "packet_size - size_left" here?

Because we want to write the size of codec data - and _not_ the size of (codec
data + padding). That's why we must remove size_left (= padding).

The funny thing is that this bug also is found in the gstreamer's asfdemuxer.
I'll attach a quick patch to fix the asfdemuxer as well. I don't know if we
should open a new bug on gstreamer-ugly, but I guess you can just commit a fix
when you find a solution you like.
Comment 1 Thiago Sousa Santos 2010-01-21 18:35:45 UTC
Created attachment 151959 [details] [review]
the patch

Patch taken from 607555
Comment 2 Thiago Sousa Santos 2010-01-21 20:43:45 UTC
Reading the asf spec again here, it is not really clear about the packet length field. But when it "talks" about padding, it says it can be deduced from the packet length. So this change sort of makes sense.
Comment 3 Thiago Sousa Santos 2010-01-22 19:15:07 UTC
Module: gst-plugins-ugly
Branch: master
Commit: 8f60eb26f3c76a3ea825422603a2a3098a0a336d
URL:    http://cgit.freedesktop.org/gstreamer/gst-plugins-ugly/commit/?id=8f60eb26f3c76a3ea825422603a2a3098a0a336d

Author: Thiago Santos <thiago.sousa.santos@collabora.co.uk>
Date:   Fri Jan 22 15:40:28 2010 -0300

asfdemux: Do not subtract padding twice

Only subtract implicit padding if an explicit one isn't
provided. Avoids subtracting it twice and causing
parsing errors.

Fixes #607698
Comment 4 Thiago Sousa Santos 2010-01-22 19:15:34 UTC
Comment on attachment 151959 [details] [review]
the patch

Committed simpler version