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 765079 - srtpdec request-key is not signaled if roc is missing
srtpdec request-key is not signaled if roc is missing
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.8.0
Other Linux
: Normal normal
: 1.8.1
Assigned To: Josep Torra Valles
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-04-15 05:31 UTC by Aleix Conchillo Flaqué
Modified: 2018-10-17 22:37 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
look for "roc" in caps (1.50 KB, patch)
2016-04-15 05:35 UTC, Aleix Conchillo Flaqué
committed Details | Review

Description Aleix Conchillo Flaqué 2016-04-15 05:31:05 UTC
Currently, gst_srtp_dec_sink_setcaps is happy if the "roc" field is not provided in the caps. If it is not provided the stream will be properly inserted in the hash table with a default "roc". Then, when the first buffer arrives validate_buffer will find an existing stream in the hash table and will not signal request-key, not allowing the user to provide a "roc".
Comment 1 Aleix Conchillo Flaqué 2016-04-15 05:35:01 UTC
Created attachment 326059 [details] [review]
look for "roc" in caps
Comment 2 Josep Torra Valles 2016-04-15 13:39:00 UTC
Comment on attachment 326059 [details] [review]
look for "roc" in caps

commit 73ebdb888e047b14ceea19ce1a0bbbeff0cd7b2a
Author: Aleix Conchillo Flaqué <aconchillo@gmail.com>
Date:   Thu Apr 14 22:32:05 2016 -0700

    srtpdec: also check for "roc" in caps
    
    Currently, gst_srtp_dec_sink_setcaps is happy if the "roc" field is not
    provided in the caps. If it is not provided the stream will be properly
    inserted in the hash table with a default "roc". Then, when the first
    buffer arrives validate_buffer will find an existing stream in the hash
    table and will not signal request-key, not allowing the user to provide
    a "roc".
    
    This patch expects "roc" in gst_srtp_dec_sink_setcaps, if not found a
    request-key will be signaled and the user will be able to provide all
    the srtp fields, including "roc".
    
    https://bugzilla.gnome.org/show_bug.cgi?id=765079
Comment 3 Olivier Crête 2018-10-15 21:36:31 UTC
@Josep I'm not sure if this is the right fix.. Shouldn't you just not be including the key at all in the caps, forcing the element to do the request? I'm not sure why you put the key in the caps in the first place if you need the ROC to make sense of it.

I'm tempted to just revert this and the related patch from bug #786304.

This break "srtpenc ! srtpdec! as srtpenc doesn't put the roc in it's caps!
Comment 4 Aleix Conchillo Flaqué 2018-10-15 21:54:29 UTC
(In reply to Olivier Crête from comment #3)
> @Josep I'm not sure if this is the right fix.. Shouldn't you just not be
> including the key at all in the caps, forcing the element to do the request?
> I'm not sure why you put the key in the caps in the first place if you need
> the ROC to make sense of it.
> 
> I'm tempted to just revert this and the related patch from bug #786304.
> 
> This break "srtpenc ! srtpdec! as srtpenc doesn't put the roc in it's caps!

I guess this comment is for me, not Josep. I don't recall much about this, but it doesn't add the key in the caps, only ROC. If ROC is not found it will just request the key (and the rest of needed fields). But certainly, it breaks "strpenc ! strpdec".

The documentation is correct though:

'''
If no rollover counter is provided by the user, 0 is used by default.
'''

Looking at it, it feels that removing this patch should just be ok because we will provide the "roc" when "request-key" is emitted.
Comment 5 Olivier Crête 2018-10-17 22:37:19 UTC
Reverted the patch and the relevant documentation change, just don't put a key in the caps if you want to do the roc manipulation yourself!