Re: [PATCH] Complete PLI handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx> 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@xxxxxxxxx
> <mailto:saghul@xxxxxxxxx>> 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@xxxxxxxxxxxxxxx
> http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org
>


--
Saúl
_______________________________________________
Visit our blog: http://blog.pjsip.org

pjsip mailing list
pjsip@xxxxxxxxxxxxxxx
http://lists.pjsip.org/mailman/listinfo/pjsip_lists.pjsip.org

[Index of Archives]     [Asterisk Users]     [Asterisk App Development]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [Linux API]
  Powered by Linux