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 751170 - dashdemux: the bitstreamSwitching attribute from AdaptationSet is not parsed
dashdemux: the bitstreamSwitching attribute from AdaptationSet is not parsed
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 1.5.90
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 752061
 
 
Reported: 2015-06-18 17:13 UTC by Florin Apostol
Modified: 2015-08-16 13:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Added parsing of bitstreamSwitching attribute and unit test to validate it. (4.94 KB, patch)
2015-06-18 17:14 UTC, Florin Apostol
committed Details | Review

Description Florin Apostol 2015-06-18 17:13:10 UTC
The bitstreamSwitching attribute from AdaptationSet is not parsed and its value is always false.
Comment 1 Florin Apostol 2015-06-18 17:14:54 UTC
Created attachment 305616 [details] [review]
Added parsing of bitstreamSwitching attribute and unit test to validate it.

Attached patch to parse the attribute and unit test to reproduce the problem.
Comment 2 Thiago Sousa Santos 2015-06-18 20:09:57 UTC
It seems we don't use bitstreamSwitching anywhere.

I tried reading what it means from the spec but it isn't very clear. Is it an extra segment that needs to be downloaded and used when switching to a track?
Comment 3 Florin Apostol 2015-06-19 09:22:12 UTC
I don't know what it is used for. My goal is to create unit tests to see if the dash parser implementation follows the standard and is able to parse any fields.
From this point of view, I consider that the implementation must be able to parse this field, in case some user will sometimes want to use it.
Comment 4 Tim-Philipp Müller 2015-06-19 09:32:54 UTC
> I tried reading what it means from the spec but it isn't
> very clear. Is it an extra segment that needs to be
> downloaded and used when switching to a track?

Sounds like it, from 5.3.3.2, although it's not clear if they always need to be downloaded or only when switching. I'm guessing when switching otherwise it would be a bit pointless.

Generally speaking it's not clear to me we need to parse stuff we don't implement, especially if it's not even clear what it's used for and whether it needs to be handled or not.
Comment 5 Florin Apostol 2015-06-19 09:51:17 UTC
I see "parsing" and "using" 2 separate things: 
- the parser scope is to implement the ability to correctly extract data from the xml. All the data mentioned in the standard. It is not its decision what it is important and what not.
- the "using" code can decide what parsed information is used and what is not used. It's scope is to process the information extracted by the parser and provide this information to the user of the gstmpdparser API.

The 2 parts can thus be logically separated.

The parsing API is the gst_mpd_parse function. The "using" API are all the other functions from gstmpdparser.h file
Comment 6 Sebastian Dröge (slomo) 2015-07-07 11:09:07 UTC
I think it makes sense to parse this, because we already have the struct field for it since forever. If the struct field exists, it should at least contain the correct value.


If the struct field should exist before we use it is another question though. Merged this for now:

commit cc9885ef4e13866c0eafe8e50de7f4af3f4f4f8e
Author: Florin Apostol <florin.apostol@oregan.net>
Date:   Fri Jul 3 16:17:58 2015 +0100

    dashdemux: added parsing of bitstreamSwitching@AdaptationSet
    
    https://bugzilla.gnome.org/show_bug.cgi?id=751170