Bug 419561 - PATCH: plparser: don't add ASX entries with a duration of 00:00:00 to the playlist
PATCH: plparser: don't add ASX entries with a duration of 00:00:00 to the pla...
Status: CLOSED ERRATA
Product: Fedora
Classification: Fedora
Component: totem (Show other bugs)
8
All Linux
low Severity low
: ---
: ---
Assigned To: Bastien Nocera
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2007-12-11 06:04 EST by Hans de Goede
Modified: 2007-12-26 20:02 EST (History)
1 user (show)

See Also:
Fixed In Version: 2.20.1-2.fc8
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2007-12-26 20:02:02 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Patch: don't add ASX entries with a duration of 00:00:00 to the playlist (917 bytes, patch)
2007-12-11 06:04 EST, Hans de Goede
no flags Details | Diff
The same patch for 2.21.3 (882 bytes, patch)
2007-12-11 06:05 EST, Hans de Goede
no flags Details | Diff
Proposed F-8 patch (2.84 KB, patch)
2007-12-12 11:19 EST, Hans de Goede
no flags Details | Diff

  None (edit)
Description Hans de Goede 2007-12-11 06:04:35 EST
When trying to play this playlist in totem:
http://www.rtl.nl/(channel=rtlnl,progid=rtlnl,zone=rtlgemist.rtl.nl/rtlnl,vm=/experience/rtlnl/,ord=1197369880870)/system/video/wvx/components/shows/idols4/video/miMedia/2007/week49/vr_nathalie_achteraf.avi_plain.xml/1537.wvx

Totem stops at the first entry, the first entry is an asx file whose first entry
is an ENTRYREF, which refers to this file:
<ASX version="3.0">
<ENTRY>
<DURATION VALUE="00:00:00" />
<TITLE />
<AUTHOR>RTL Nederland</AUTHOR>
<REF HREF="http://www.rtl.nl/system/transparant-pixel.gif" />
</ENTRY>
</ASX>

Totem then tries to interpret to transparant-pixel.gif as an asf stream, fails
and stops. This is especially a problem when using the plugin (which is the
normal way to get to these streams) as with the plugin one cannot get to the
playlist to switch to the second entry of the playlist.

I've written the attached patch for this which simply ignores ASX entries with a
duration of 00:00:00, which seems like the right thing todo. I guess this entry
is only there to track the number of views.
Comment 1 Hans de Goede 2007-12-11 06:04:35 EST
Created attachment 283941 [details]
Patch: don't add ASX entries with a duration of 00:00:00 to the playlist
Comment 2 Hans de Goede 2007-12-11 06:05:15 EST
Created attachment 283951 [details]
The same patch for 2.21.3
Comment 3 Bastien Nocera 2007-12-11 06:21:28 EST
No, this should be done in the front-ends, not the parser itself.
The browser plugin already has such code, we'd need that code in the main Totem
itself (see src/totem-playlist.c).
Comment 4 Hans de Goede 2007-12-11 06:40:11 EST
(In reply to comment #3)
> No, this should be done in the front-ends, not the parser itself.

Hmm, why? Then the wheel needs to get reinvented each time, anyways if you
really want it that way I'll happily adapt my patch to make it so.

> The browser plugin already has such code, we'd need that code in the main Totem
> itself (see src/totem-playlist.c).

Well it doesn't seem to have such code for this case as opening for example:
http://www.rtl.nl/(vm=/service/gemist/home/,id_by_component)/system/video/html/components/reality/rtlprojectcatwalk/miMedia/2007/week50/ma_projectcatwalk_10dec.avi_plain.xml

Where the plugin gets loaded, doesn't work (not even when all the ugly / bad
ffmpeg plugins needed are present), and with my plparser patch it does, or am I
misunderstanding and does src/totem-playlist.c get used by both the plugin and
the player?

Also src/totem-playlist.c is not a small file I've taken a quick look but I
couldn't find an obvious place to add a check for 00:00:00 duration files, which
function should I look at?
Comment 5 Bastien Nocera 2007-12-11 09:13:13 EST
$ ./test-parser file:///tmp/totem/browser-plugin/1537.wvx 

###################### parsing ################

added URI 'http://www.rtl.nl/system/transparant-pixel.gif'
        duration = '00:00:00'
        author = 'RTL Nederland'
added URI
'mmsh://idols-av.rtl.nl/web/components/shows/idols4/video/2007/week49/vr_nathalie_achteraf.avi.MiMedia_WM_1500K_V9_idols-av_film_an_t4b4l2r2_716x402.wmv?MSWMExt=.asf'

And in browser-plugin/totem-plugin-viewer.c:
       /* Skip short advert streams */
       duration = totem_pl_parser_parse_duration (g_hash_table_lookup (metadata,
"duration"), FALSE);
       if (duration == 0)
              return;

I also noticed that we already ignore 0-length items in playlists in
totem_playlist_entry_parsed() in trunk.

So it's already fixed in rawhide (despite what you put in the version field for
the report). Feel free to add your work-around to F-8 though.
Comment 6 Hans de Goede 2007-12-12 11:16:36 EST
(In reply to comment #5)
> So it's already fixed in rawhide (despite what you put in the version field for
> the report).

Sorry about that.

> Feel free to add your work-around to F-8 though.
You mean the attached patch for plparser you didn't want upstream? I guess it
would be better then to just add these 2 patches from upstream:
http://svn.gnome.org/viewvc/totem/trunk/browser-plugin/totem-plugin-viewer.c?r1=4838&r2=4866
http://svn.gnome.org/viewvc/totem/trunk/browser-plugin/totem-plugin-viewer.c?r1=4838&r2=4866

(With some adjustments to apply and compile properly)

Shall I add those 2 to F-8, commit build and push to updates-testing?
Comment 7 Hans de Goede 2007-12-12 11:19:37 EST
Created attachment 285851 [details]
Proposed F-8 patch

Here is a proposed patch for F-8 based on the upstream fix for this.
Comment 8 Bastien Nocera 2007-12-12 11:31:14 EST
(In reply to comment #6)
> (In reply to comment #5)
> > So it's already fixed in rawhide (despite what you put in the version field for
> > the report).
> 
> Sorry about that.

No worries, happens to me as well.

> > Feel free to add your work-around to F-8 though.
> You mean the attached patch for plparser you didn't want upstream?

> I guess it
> would be better then to just add these 2 patches from upstream:
>
http://svn.gnome.org/viewvc/totem/trunk/browser-plugin/totem-plugin-viewer.c?r1=4838&r2=4866
>
http://svn.gnome.org/viewvc/totem/trunk/browser-plugin/totem-plugin-viewer.c?r1=4838&r2=4866
> 
> (With some adjustments to apply and compile properly)
> 
> Shall I add those 2 to F-8, commit build and push to updates-testing?

Yeah, looks fine actually. Thanks!

I committed that patch to the 2.20 branch, so we can remove it next time I make
a release.
Comment 9 Hans de Goede 2007-12-12 16:11:26 EST
Patch committed, build and push to updates-testing required.
Comment 10 Fedora Update System 2007-12-13 17:16:47 EST
totem-2.20.1-2.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update totem'
Comment 11 Fedora Update System 2007-12-23 17:50:12 EST
totem-2.20.1-2.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update totem'
Comment 12 Fedora Update System 2007-12-26 18:50:45 EST
totem-2.20.1-2.fc8 has been pushed to the Fedora 8 testing repository.  If problems still persist, please make note of it in this bug report.
 If you want to test the update, you can install it with 
 su -c 'yum --enablerepo=updates-testing update totem'
Comment 13 Fedora Update System 2007-12-26 20:02:01 EST
totem-2.20.1-2.fc8 has been pushed to the Fedora 8 stable repository.  If problems still persist, please make note of it in this bug report.

Note You need to log in before you can comment on or make changes to this bug.