[PATCH] Complete PLI handling
Hey there,
Good to be back hacking on PJSIP :-)
I noticed some nice improvements in parsing RTCP feedback packets, nice!
Full PLI support looked to be in the TODO list, and since we (Jitsi)
need it for a project, here are some patches which complete PLI support
by building on top of the existing APIs and adding where needed.
Patch 001: makes vid_stream automagically send a keyframe when a PLI is
received.
Patch 002: adds an API to vid_stream in order to send a PLI packet.
Patch 003: make pjsua (also) send PLI packets when a keyframe is missing.
Cheers,
--
Saúl
Hi Saúl,
Actually we've also just implemented it, please check
https://trac.pjsip.org/repos/ticket/1437. I've checked your patch and I
think all the features have also covered by the ticket (please let us know
if something is missing), perhaps the difference is just who do the RTCP
sending, instead of PJSUA-LIB (side by side with the SIP INFO method), the
ticket does it in video stream. Also the ticket includes SDP attribute
generation to signal RTCP-FB PLI capability.
Anyway, thank you so much for sharing patches, please keep sharing :)
BR,
nanang
On Wed, Dec 4, 2019 at 5:27 PM Saúl Ibarra Corretgé saghul@jitsi.org
wrote:
Hey there,
Good to be back hacking on PJSIP :-)
I noticed some nice improvements in parsing RTCP feedback packets, nice!
Full PLI support looked to be in the TODO list, and since we (Jitsi)
need it for a project, here are some patches which complete PLI support
by building on top of the existing APIs and adding where needed.
Patch 001: makes vid_stream automagically send a keyframe when a PLI is
received.
Patch 002: adds an API to vid_stream in order to send a PLI packet.
Patch 003: make pjsua (also) send PLI packets when a keyframe is missing.
Cheers,
--
Saúl
Visit our blog: http://blog.pjsip.org
pjsip mailing list
pjsip@lists.pjsip.org
http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
Hi Nanang!
On 06/12/2019 10:49, Nanang Izzuddin wrote:
Hi Saúl,
Actually we've also just implemented it, please check
https://trac.pjsip.org/repos/ticket/1437. I've checked your patch and I
think all the features have also covered by the ticket (please let us
know if something is missing), perhaps the difference is just who do the
RTCP sending, instead of PJSUA-LIB (side by side with the SIP INFO
method), the ticket does it in video stream. Also the ticket includes
SDP attribute generation to signal RTCP-FB PLI capability.
Anyway, thank you so much for sharing patches, please keep sharing :)
Oh this is excellent news! You even went ahead and added NACK support!
This is fantastic!
I missed it because I based our work on latest master from the GitHub
mirror, which seems to be behind. Any chance you can update it? It would
make keeping our work (mostly haccks which are not worth sharing) easier :-)
Thanks a lot!
Cheers,
BR,
nanang
On Wed, Dec 4, 2019 at 5:27 PM Saúl Ibarra Corretgé <saghul@jitsi.org
mailto:saghul@jitsi.org> wrote:
Hey there,
Good to be back hacking on PJSIP :-)
I noticed some nice improvements in parsing RTCP feedback packets, nice!
Full PLI support looked to be in the TODO list, and since we (Jitsi)
need it for a project, here are some patches which complete PLI support
by building on top of the existing APIs and adding where needed.
Patch 001: makes vid_stream automagically send a keyframe when a PLI is
received.
Patch 002: adds an API to vid_stream in order to send a PLI packet.
Patch 003: make pjsua (also) send PLI packets when a keyframe is
missing.
Cheers,
--
Saúl
_______________________________________________
Visit our blog: http://blog.pjsip.org
pjsip mailing list
pjsip@lists.pjsip.org <mailto:pjsip@lists.pjsip.org>
http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
Visit our blog: http://blog.pjsip.org
pjsip mailing list
pjsip@lists.pjsip.org
http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
--
Saúl
On 06/12/2019 10:49, Nanang Izzuddin wrote:
Hi Saúl,
Actually we've also just implemented it, please check
https://trac.pjsip.org/repos/ticket/1437. I've checked your patch and I
think all the features have also covered by the ticket (please let us
know if something is missing), perhaps the difference is just who do the
RTCP sending, instead of PJSUA-LIB (side by side with the SIP INFO
method), the ticket does it in video stream. Also the ticket includes
SDP attribute generation to signal RTCP-FB PLI capability.
Hi again Nanag!
I just spent some time rebasing my changes on top of latest master
(thanks for syncing up GitHub, it make my life heaps easier!). Great
work folks, most of my patches are no longer necessary!
There is, however, one thing whicch AFAICT is not currently implemented
by PJSUA: sending a PLI when PJMEDIA_EVENT_KEYFRAME_MISSING is received
and PJSUA_VID_REQ_KEYFRAME_RTCP_PLI is set.
The attached patches add this functionality. The first one exposes a
pjmedia_vid_stream_send_rtcp_pli function which the second patch uses.
Please let me know what you think and if you need me to make any
changes. Keeping our patches at the minimum is my goal :-)
Cheers,
--
Saúl
On 14/02/2020 15:46, Saúl Ibarra Corretgé wrote:
On 06/12/2019 10:49, Nanang Izzuddin wrote:
Hi Saúl,
Actually we've also just implemented it, please check
https://trac.pjsip.org/repos/ticket/1437. I've checked your patch and I
think all the features have also covered by the ticket (please let us
know if something is missing), perhaps the difference is just who do the
RTCP sending, instead of PJSUA-LIB (side by side with the SIP INFO
method), the ticket does it in video stream. Also the ticket includes
SDP attribute generation to signal RTCP-FB PLI capability.
Hi again Nanag!
I just spent some time rebasing my changes on top of latest master
(thanks for syncing up GitHub, it make my life heaps easier!). Great
work folks, most of my patches are no longer necessary!
There is, however, one thing whicch AFAICT is not currently implemented
by PJSUA: sending a PLI when PJMEDIA_EVENT_KEYFRAME_MISSING is received
and PJSUA_VID_REQ_KEYFRAME_RTCP_PLI is set.
The attached patches add this functionality. The first one exposes a
pjmedia_vid_stream_send_rtcp_pli function which the second patch uses.
Please let me know what you think and if you need me to make any
changes. Keeping our patches at the minimum is my goal :-)
Cheers,
Now you folks have moved fully to GH (awesome!) I send thesse as a PR:
https://github.com/pjsip/pjproject/pull/2286
--
Saúl
Hi Saúl,
Actually the library is supposed to also automatically send PLI on missing
keyframe, but currently it happens only if remote signals RTCP-FB PLI in
their SDP. So it seems that you are suggesting to always send PLI
regardless remote signalling it or not (as long
as PJSUA_VID_REQ_KEYFRAME_RTCP_PLI is set), well, it does not seem to be
prohibited by the standard, so why not :) Anyway, I think the approach in
the attached patch should be a bit better as sending PLI is done in PJMEDIA
level, note that the SIP INFO is done in PJSUA level because PJMEDIA cannot
send SIP signalling. The patch is untested though :)
BR,
nanang
On Tue, Feb 18, 2020 at 5:58 PM Saúl Ibarra Corretgé saghul@jitsi.org
wrote:
On 14/02/2020 15:46, Saúl Ibarra Corretgé wrote:
On 06/12/2019 10:49, Nanang Izzuddin wrote:
Hi Saúl,
Actually we've also just implemented it, please check
https://trac.pjsip.org/repos/ticket/1437. I've checked your patch and I
think all the features have also covered by the ticket (please let us
know if something is missing), perhaps the difference is just who do the
RTCP sending, instead of PJSUA-LIB (side by side with the SIP INFO
method), the ticket does it in video stream. Also the ticket includes
SDP attribute generation to signal RTCP-FB PLI capability.
Hi again Nanag!
I just spent some time rebasing my changes on top of latest master
(thanks for syncing up GitHub, it make my life heaps easier!). Great
work folks, most of my patches are no longer necessary!
There is, however, one thing whicch AFAICT is not currently implemented
by PJSUA: sending a PLI when PJMEDIA_EVENT_KEYFRAME_MISSING is received
and PJSUA_VID_REQ_KEYFRAME_RTCP_PLI is set.
The attached patches add this functionality. The first one exposes a
pjmedia_vid_stream_send_rtcp_pli function which the second patch uses.
Please let me know what you think and if you need me to make any
changes. Keeping our patches at the minimum is my goal :-)
Cheers,
Now you folks have moved fully to GH (awesome!) I send thesse as a PR:
https://github.com/pjsip/pjproject/pull/2286
--
Saúl
On 19/02/2020 12:14, Nanang Izzuddin wrote:
Hi Saúl,
Actually the library is supposed to also automatically send PLI on
missing keyframe, but currently it happens only if remote signals
RTCP-FB PLI in their SDP. So it seems that you are suggesting to always
send PLI regardless remote signalling it or not (as long
as PJSUA_VID_REQ_KEYFRAME_RTCP_PLI is set), well, it does not seem to be
prohibited by the standard, so why not :) Anyway, I think the approach
in the attached patch should be a bit better as sending PLI is done in
PJMEDIA level, note that the SIP INFO is done in PJSUA level because
PJMEDIA cannot send SIP signalling. The patch is untested though :)
BR,
nanang
Ah, you're right! I hadn't noticed the handling of
PJMEDIA_EVENT_KEYFRAME_MISSING in vid_stream.c ignore my patches then!
On a tangentially related topic, I have a custom patch (which I don't
think would be acceptable for PJSIP) which requests a keyframe at
regular intervals, on a timer. Any ideas on how to force send a PLI
without my "pjmedia_vid_stream_send_rtcp_pli" patch? Maybe that part
still has some value?
Cheers,
On Tue, Feb 18, 2020 at 5:58 PM Saúl Ibarra Corretgé <saghul@jitsi.org
mailto:saghul@jitsi.org> wrote:
On 14/02/2020 15:46, Saúl Ibarra Corretgé wrote:
On 06/12/2019 10:49, Nanang Izzuddin wrote:
Hi Saúl,
Actually we've also just implemented it, please check
https://trac.pjsip.org/repos/ticket/1437. I've checked your patch
and I
think all the features have also covered by the ticket (please let us
know if something is missing), perhaps the difference is just who
do the
RTCP sending, instead of PJSUA-LIB (side by side with the SIP INFO
method), the ticket does it in video stream. Also the ticket includes
SDP attribute generation to signal RTCP-FB PLI capability.
Hi again Nanag!
I just spent some time rebasing my changes on top of latest master
(thanks for syncing up GitHub, it make my life heaps easier!). Great
work folks, most of my patches are no longer necessary!
There is, however, one thing whicch AFAICT is not currently
implemented
by PJSUA: sending a PLI when PJMEDIA_EVENT_KEYFRAME_MISSING is
received
and PJSUA_VID_REQ_KEYFRAME_RTCP_PLI is set.
The attached patches add this functionality. The first one exposes a
pjmedia_vid_stream_send_rtcp_pli function which the second patch uses.
Please let me know what you think and if you need me to make any
changes. Keeping our patches at the minimum is my goal :-)
Cheers,
Now you folks have moved fully to GH (awesome!) I send thesse as a PR:
https://github.com/pjsip/pjproject/pull/2286
--
Saúl
Visit our blog: http://blog.pjsip.org
pjsip mailing list
pjsip@lists.pjsip.org
http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
--
Saúl
Right, perhaps we can merge the pjmedia_vid_stream_send_rtcp_pli() part.
A GitHub newbie here :D I guess the next step would be, could you please
clean up the PR to only contain pjmedia_vid_stream_send_rtcp_pli() and
perhaps along with my proposed patch (tested :)?
And let's continue the discussion in the PR.
BR,
nanang
On Wed, Feb 19, 2020 at 7:47 PM Saúl Ibarra Corretgé saghul@jitsi.org
wrote:
On 19/02/2020 12:14, Nanang Izzuddin wrote:
Hi Saúl,
Actually the library is supposed to also automatically send PLI on
missing keyframe, but currently it happens only if remote signals
RTCP-FB PLI in their SDP. So it seems that you are suggesting to always
send PLI regardless remote signalling it or not (as long
as PJSUA_VID_REQ_KEYFRAME_RTCP_PLI is set), well, it does not seem to be
prohibited by the standard, so why not :) Anyway, I think the approach
in the attached patch should be a bit better as sending PLI is done in
PJMEDIA level, note that the SIP INFO is done in PJSUA level because
PJMEDIA cannot send SIP signalling. The patch is untested though :)
BR,
nanang
Ah, you're right! I hadn't noticed the handling of
PJMEDIA_EVENT_KEYFRAME_MISSING in vid_stream.c ignore my patches then!
On a tangentially related topic, I have a custom patch (which I don't
think would be acceptable for PJSIP) which requests a keyframe at
regular intervals, on a timer. Any ideas on how to force send a PLI
without my "pjmedia_vid_stream_send_rtcp_pli" patch? Maybe that part
still has some value?
Cheers,
On Tue, Feb 18, 2020 at 5:58 PM Saúl Ibarra Corretgé <saghul@jitsi.org
mailto:saghul@jitsi.org> wrote:
On 14/02/2020 15:46, Saúl Ibarra Corretgé wrote:
On 06/12/2019 10:49, Nanang Izzuddin wrote:
Hi Saúl,
Actually we've also just implemented it, please check
https://trac.pjsip.org/repos/ticket/1437. I've checked your patch
and I
think all the features have also covered by the ticket (please
let us
know if something is missing), perhaps the difference is just who
do the
RTCP sending, instead of PJSUA-LIB (side by side with the SIP INFO
method), the ticket does it in video stream. Also the ticket
includes
SDP attribute generation to signal RTCP-FB PLI capability.
Hi again Nanag!
I just spent some time rebasing my changes on top of latest master
(thanks for syncing up GitHub, it make my life heaps easier!).
Great
work folks, most of my patches are no longer necessary!
There is, however, one thing whicch AFAICT is not currently
implemented
by PJSUA: sending a PLI when PJMEDIA_EVENT_KEYFRAME_MISSING is
received
and PJSUA_VID_REQ_KEYFRAME_RTCP_PLI is set.
The attached patches add this functionality. The first one exposes
a
pjmedia_vid_stream_send_rtcp_pli function which the second patch
uses.
Please let me know what you think and if you need me to make any
changes. Keeping our patches at the minimum is my goal :-)
Cheers,
Now you folks have moved fully to GH (awesome!) I send thesse as a
PR:
https://github.com/pjsip/pjproject/pull/2286
--
Saúl
Visit our blog: http://blog.pjsip.org
pjsip mailing list
pjsip@lists.pjsip.org
http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
--
Saúl