[jitsi-dev] [PATCH] Set default max video bandwidth to 128


#1

Here is the second patch.

Since the previous patch gets user defined bitrate, it sets default
bandwidth to 128 to keep previous behavior by default.

02-default_video_bandwidth.patch (560 Bytes)


#2

Hi,

this naming is a bit ambiguous.
It should be defined one default MAX bandwith
and a default bandwith. The name suggests
that this is a max bandwidth and this should not
be limited to 128 by default.

Cheers,
Matt

···

Am 26.02.2013 11:16, schrieb Benoît Laniel:

Here is the second patch.

Since the previous patch gets user defined bitrate, it sets default
bandwidth to 128 to keep previous behavior by default.


#3

I did not change any name.

The situation was:
- Allow user to define a MAX bandwidth (and don't use it)
- Hardcode 128Kbps for h264, 192kbps for vp8 and 256kbps for h263
So MAX bandwidth does not have any effect.

With the previous patch:
- Allow user to define a MAX bandwidth
- Use user defined MAX bandwidth to define bitrate for x264 and vp8
For x264, bitrate in crf mode is effectively the MAX bandwidth. From my
tests, the same applies to vp8.

This patch is to allow the user to choose the MAX bandwidth while
keeping previous behavior (hardcoded 128kbps for h264) by default. But
we can keep a default of 256kbps (which is still hardcoded in h263, I
did not have time to investigate the necessary changes).

Defining a default bandwidth is not needed imo since it would use too
much bandwidth for nothing.

Benoît

···

Le mardi 26 février 2013 à 12:22 +0100, Buddy Butterfly a écrit :

Hi,

this naming is a bit ambiguous.
It should be defined one default MAX bandwith
and a default bandwith. The name suggests
that this is a max bandwidth and this should not
be limited to 128 by default.

Cheers,
Matt

Am 26.02.2013 11:16, schrieb Benoît Laniel:
> Here is the second patch.
>
> Since the previous patch gets user defined bitrate, it sets default
> bandwidth to 128 to keep previous behavior by default.


#4

Hello and thank you for the patches!

We discussed this with Emil today, and while the functionality is something
we would like to have, there is a slight problem with this implementation.

The option that we currently have in the GUI is being used, but in a
different way. It's used to limit the rate of outgoing RTP packets to avoid
bursts. See VideoMediaStreamImpl#configureDataOutputStream()

We would like to have a separate option in the GUI (and a separate
property) which controls the bitrate of the encoders in the way that you
propose. Would it be possible for you to update your patch to take this
approach?

Regards,
Boris


#5

Hello and thank you for the patches!

We discussed this with Emil today, and while the functionality is
something we would like to have, there is a slight problem with this
implementation.

The option that we currently have in the GUI is being used, but in a
different way. It's used to limit the rate of outgoing RTP packets to
avoid bursts. See VideoMediaStreamImpl#configureDataOutputStream()

Indeed I misunderstood the meaning of MAX_BANDWIDTH. I thought it was
aimed at the codec and completely forgot to check network code.

We would like to have a separate option in the GUI (and a separate
property) which controls the bitrate of the encoders in the way that
you propose. Would it be possible for you to update your patch to take
this approach?

I'll try do it. Maybe an option like MAX_BITRATE or MAX_CODEC_BITRATE
would have sense ? However, I think the difference between the words
bandwidth and bitrate is a bit confusing (especially for people not used
to computers). Any suggestions ?

Regards,
Benoît

···

Le mardi 26 février 2013 à 19:13 +0200, Boris Grozev a écrit :


#6

Here is a first draft of the updated patch(es). As suggested by Matt, I
used BITRATE instead of MAX_BITRATE to (try to) avoid confusion with
MAX_BANDWIDTH.

It allows control on H263, H264 and VP8 bitrates. Please note that old
defaults were 256 for H263, 128 for H264 and 192 for VP8.

Regards,
Benoît

libjitsi_video_bitrate.patch (5.95 KB)

jitsi_video_bitrate.patch (3.88 KB)

···

Le mardi 26 février 2013 à 19:56 +0100, Benoît Laniel a écrit :

> We would like to have a separate option in the GUI (and a separate
> property) which controls the bitrate of the encoders in the way that
> you propose. Would it be possible for you to update your patch to take
> this approach?

I'll try do it. Maybe an option like MAX_BITRATE or MAX_CODEC_BITRATE
would have sense ? However, I think the difference between the words
bandwidth and bitrate is a bit confusing (especially for people not used
to computers). Any suggestions ?


#7

Hey Benoît,

I'll try do it. Maybe an option like MAX_BITRATE or MAX_CODEC_BITRATE
would have sense ?

Yup, SGTM.

However, I think the difference between the words
bandwidth and bitrate is a bit confusing (especially for people not used
to computers). Any suggestions ?

It is definitely confusing since there's nothing in the word BANDWIDTH
that implies what we are really doing. Fortunately, the names of these
properties are not visible to users so it would be enough to just make
sure we show the right string in the GUI.

That said, if you feel like changing the names we can keep MAX_BITRATE
for the new feature and change the existing one to something like
RTP_PACING_THRESHOLD.

Cheers,
Emil

···

On 26.02.13, 20:56, Benoît Laniel wrote:

--
https://jitsi.org


#8

Hello, Benoît

Thanks again for the contribution and apologies for the long delay.

I've committed a slightly modified version of your patches. The changes are
part of revision 10665 and are ack'ed on the website.

I renamed some methods and variables to avoid using "max bandwidth" (using
an "RTP pacing threshold" theme instead), but kept the names of the
properties.

Regards,
Boris