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 671073 - Fix rtpvp8 compatibility with the latest draft
Fix rtpvp8 compatibility with the latest draft
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal normal
: 0.10.24
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2012-02-29 16:39 UTC by Danilo Cesar
Modified: 2012-03-08 20:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch (2.51 KB, patch)
2012-02-29 16:39 UTC, Danilo Cesar
none Details | Review
patch (2.26 KB, patch)
2012-02-29 17:37 UTC, Danilo Cesar
none Details | Review
patch (1.42 KB, patch)
2012-03-01 18:01 UTC, Danilo Cesar
none Details | Review
patch (1.42 KB, patch)
2012-03-01 18:02 UTC, Danilo Cesar
committed Details | Review

Description Danilo Cesar 2012-02-29 16:39:55 UTC
Created attachment 208700 [details] [review]
patch

Making it compatible with the 03 spec

http://tools.ietf.org/html/draft-ietf-payload-vp8-03
Comment 1 Tim-Philipp Müller 2012-02-29 17:00:52 UTC
Just to check - it's intentional that the older encoding-name(s) in the depayloader are no longer accepted, right?
Comment 2 Danilo Cesar 2012-02-29 17:37:14 UTC
Created attachment 208703 [details] [review]
patch
Comment 3 Danilo Cesar 2012-02-29 17:45:22 UTC
Had a discussion with Tim on the irc and we went to a few questions:

Can we ignore the old drafts and keep it as it is? From my understanding we can't since empathy uses it as the main video codec, otherwise a user running 01 won't be able to chat with someone running 03, even if the code is fully compatible.

Tim suggested that we could use 
encoding-name = (string) { VP8-DRAFT-IETF-01, VP8-DRAFT-IETF-02, VP8-DRAFT-IETF-03 } 
and then realized it's not supported. So, is there a way to say on the encoding- name that we support all the drafts? Should we do it?

* fixed the submodule and pushing a new file.
Comment 4 Tim-Philipp Müller 2012-02-29 17:47:53 UTC
> Tim suggested that we could use 
> encoding-name = (string) { VP8-DRAFT-IETF-01, VP8-DRAFT-IETF-02,
> VP8-DRAFT-IETF-03 } 
> and then realized it's not supported. So, is there a way to say on the
> encoding- name that we support all the drafts? Should we do it?

That should be fine for the depayloader.

The question is what to do with the payloader. The patch changes it to always output -03, and I was wondering if it needs to also support outputting the old -01 format for backwards compatibility. If so, how would that be signalled by the app? downstream caps? a property?
Comment 5 Olivier Crête 2012-02-29 17:51:15 UTC
I seems to be mostly compatible, just keep -01 ? Since we don't create K headers anyway.
Comment 6 Olivier Crête 2012-02-29 18:24:55 UTC
If there are changes in the payloader, it should definitely be negotiated with the caps.
Comment 7 Danilo Cesar 2012-03-01 18:01:38 UTC
Created attachment 208798 [details] [review]
patch

Keeping -01 version, since it's compatible with 03 but doesn't implement the K and TID headers.
Comment 8 Danilo Cesar 2012-03-01 18:02:54 UTC
Created attachment 208799 [details] [review]
patch

Keeping -01 version, since it's compatible with 03 but doesn't implement the K
and TID headers.
Comment 9 Youness Alaoui 2012-03-01 18:06:44 UTC
I agree with Olivier that the changes are compatible, and it will work from draft 1 to 3.
So maybe keep Draft-01 in the caps, and add another caps property last-compatible-draft=3 so it would mean "latest draft compatibility is 3, but last backward compatible is 1" kind of like libtool's age/revision/current.
Comment 10 Tim-Philipp Müller 2012-03-01 18:17:24 UTC
What's the plan for when it's declared stable? Still support the various drafts?
Comment 11 Olivier Crête 2012-03-01 20:46:34 UTC
I think the plan is to just remove the draft. My fear was that if we used the stable one directly we'd be stuck with stuff that claims to be compatible but isn't and would break in strange ways. If we just change the caps at the end, then we end up not trying to interop with older versions.
Comment 12 Olivier Crête 2012-03-08 20:24:14 UTC
committed

commit 35df907f93e004c8cc04e4972011b4ddeb492799
Author: Danilo Cesar Lemes de Paula <danilo.cesar@collabora.co.uk>
Date:   Thu Mar 1 14:59:55 2012 -0300

    Fixing rtpvp8 compatibility with the third draft
    
    https://bugzilla.gnome.org/show_bug.cgi?id=671073