[RFC] Issues with undershoot and overshoot and intra/inter frame percentages

46 views
Skip to first unread message

John Koleszar

unread,
Apr 5, 2011, 11:58:48 AM4/5/11
to codec...@webmproject.org, aron_ro...@logitech.com, Mikhal Shemer
Hi all,

I wanted to start a discussion on a couple rate control issues and how
they relate to the API. I'm cribbing much of this text from a related
issue[1] and change set[2], please look at those links for the
original sources.

[1]: http://code.google.com/p/webm/issues/detail?id=270
[2]: https://review.webmproject.org/#change,1890

I'm trying to understand:

1) Adding new parameters to the configuration structure breaks the
ABI/API. As our next release will be around the 1 year anniversary of
the launch, it may be reasonable to bump to version 1.0.0 and
incorporate some minor API changes. Does this sound unreasonable to
anyone?

2) Are these the *right* variables to expose to an application? Do the
proposed controls in these two patches accurately describe the most
useful variables to expose to an application, or are they "just"
working around specific problems inside the codec?

3) Are there any other variables we need to be considering here?
Changes people are working on, etc? Let's get all the cards on the
table.

What does everyone think?

Aron Rosenberg proposed a patch (Issue 270)[1]:

The VPX codec has many hard coded constraints/parameters that control
the minimum and maximum sizes of the per frame targets for both inter
and intra encoding. The problem with these constraints is the
following:

1) They force the size of all intra encoded frames to be excessively
large for real time encoding
2) They limit the speed at which the adjusted per-frame targets
increase or decrease in size, thus limiting the speed at which rate
control can adapt video to the target data rate.
3) The codec will often under or over-shoot the rate, based on
content, and not recover quickly or obey the buffer constraints
4) The minimum frame size is hardcoded, resulting in an effective
quality floor that can't be over-ridden by user choice, and resulting
in target data rates being exceeded for some content types

[...]

The attached "discussion" patch enables fixes for these issues by
adding the following four encoder level variables.

/*!\brief Rate control adaptation undershoot and overshoot tolerance
*
* This value, expressed as a percentage of the target
bitrate, controls the
* maximum allowed adaptation speed of the codec in terms of
maintaining the
* buffer size at the correct fill size in terms of a higher rate
*/
unsigned int rc_undershoot_buffer_pct;
unsigned int rc_overshoot_buffer_pct;

/*!\brief Set the percentage of the per second data rate to
allow for I-Frames
*
* This value, expressed as a percentage of the target bitrate, sets
* the default and minimum bitrate for an i-Frame as a
percentage of the kbps
* bit rate target- can be increased above the value in
error_resilient mode
*/
unsigned int rc_min_intra_pct;

/*!\brief Set the percentage of the per second data rate to
allow for I-Frames
*
* This value, expressed as a percentage of the target bitrate, sets
* the minimum allowed bitrate for a p-Frame as a percentage
of the per-frame
* target data rate
*/
unsigned int rc_min_inter_pct;


Note: (Aron) added new undershoot and overshoot variables which
precisely work so that the API would (sic?) break for people using the
existing undershoot. (Existing overshoot was defined, but never used).

Mikhal Shemer proposed this patch (Change 1890)[2]

/*!\brief Initial Key Quantizer
*
* This value allows the user to specify the quantizer value for the
* first key frame of a sequence. Set to zero to use the codec default.
*/
unsigned int rc_initial_key_quantizer;

A few quick thoughts from me:

Do we need both rc_undershoot_buffer_pct and rc_undershoot_pct?

rc_min_intra_pct: need a max too? that's the use case for
rc_initial_key_quantizer.

rc_initial_key_quantizer is a very specific control -- if we want
precise control over q selection, maybe we should have an interface to
set the q for any given frame, which would be useful for development
too. See change[3] 1306 for related comments. Or if this is just about
rate, maybe we should do something in those terms, like
rc_{min,max}_intra_pct.

[3]: https://review.webmproject.org/#change,1306


Thanks!

John


P.S. Sorry to Aron and Mikhal that this issue has gone unaddressed for so long.

Aron Rosenberg

unread,
Apr 5, 2011, 12:08:26 PM4/5/11
to codec...@webmproject.org, Mikhal Shemer, John Koleszar
Since we are on the topic of rate control now, I added another patch to the original ticket for the undershoot / overshoot which "fixes" and allows precise real-time bitrate adjustment without having other params reset. It is attached to the same issue 270.

Aron Rosenberg
Sr. Director, Engineering
Logitech Inc. (SightSpeed Group)

Mikhal Shemer

unread,
Apr 5, 2011, 6:53:30 PM4/5/11
to John Koleszar, aaron_r...@logitech.com, codec...@webmproject.org, Marco Paniconi
Hi, 

In general, I agree that there are many hard coded parameters in the VPX code, and that exposing them will make it easier for the application to 
control codec behavior, especially for real-time applications in which the constraints are considerable.

These new API's can be designed such that when not used, they fall back to the current default behavior, so not to impact applications which continue to use the existing API (perhaps not always possible) .  

I have added an API to control the quantization value for the initial key-frame (which in turn controls the size), as we have also found that the initial key frame to be too large (change set 2 below). It should be fine replacing this one with a limit on the max intra frame size. In addition, the proposed set bit-rate function clamps the key frame size to hard-coded threshold, introducing such a variable would resolve that too.    

An additional API modification that we thought of concerns the internal frame dropper. The current implementation sets a threshold which enables temporal sampling based on the buffer level. In addition,  frames are dropped whenever the buffer level goes below zero (a hard coded threshold). We would like to expose that threshold as well. It might be reasonable to rename the existing threshold to  rc_temporaldecimation_thresh, and have the new one keep the existing name: rc_dropframe_thresh. Please note that by doing so, the current structure will be modified.    
 
Both rc_under/over - shoot parameters specify a multiplier to the target frame (one more explicit than the other). I haven't tested it, but I assume that keeping the ones based on the buffer level, as in Aaron's proposal, should be better for rate control.

-Mikhal 


 
On Tue, Apr 5, 2011 at 8:58 AM, John Koleszar <jkol...@google.com> wrote:

John Koleszar

unread,
Apr 8, 2011, 11:01:40 AM4/8/11
to Mikhal Shemer, aaron_r...@logitech.com, codec...@webmproject.org, Marco Paniconi
Ok, to summarize, we've mentioned so far:

rc_undershoot_buffer_pct;
rc_overshoot_buffer_pct;
rc_min_intra_pct;
rc_min_inter_pct;
rc_max_intra_pct;
rc_initial_key_quantizer;
rc_dropframe_thresh;
rc_temporaldecimation_thresh;
vp8e_set_bitrate()
VP8E_SET_ENABLEAUTOGF
VP8E_SET_GF_FREQUENCY
VP8E_SET_ARF_FREQUENCY

I think this may get a little unwieldy to handle all in the same
thread, so let's just focus on a couple at a time. I'll make sure we
hit them all.

Let's start with these two:

rc_undershoot_buffer_pct;
rc_overshoot_buffer_pct;

My understanding of this patch is that it tries to give control over
how fast the rate targeting adapts by limiting the amount of the
difference between the current and target buffer levels is applied to
the current frame target. The existing parameters are
rc_undershoot_pct, which only applies a fixed bias to the target rate,
and rc_overshoot_pct, which is unused.

I propose that we use the existing names and the new behavior from the
patch, rather than introducing new parameters. Any objections?
Drawbacks? Another option would be to remove the current variables and
introduce these new controls under new names (as proposed or
different) to better signal to users that the semantics have changed.
My reading is that the current variables are mostly useless, so
changing the semantics (probably) isn't going to hurt anybody, and
it's not worth introducing a new name unless the new name is
better/more accurate/etc, but happy to hear other thoughts?

John Koleszar

unread,
Apr 8, 2011, 11:04:37 AM4/8/11
to Mikhal Shemer, arose...@logitech.com, codec...@webmproject.org, Marco Paniconi
(correcting cc:)

Henrik Lundin

unread,
Apr 8, 2011, 12:05:12 PM4/8/11
to codec...@webmproject.org
John,

I intend to look into this, but I haven't had the time yet. Will do it next week. 

/Henrik


(correcting cc:)
--
You received this message because you are subscribed to the Google Groups "Codec Developers" group.
To post to this group, send email to codec...@webmproject.org.
To unsubscribe from this group, send email to codec-devel...@webmproject.org.
For more options, visit this group at http://groups.google.com/a/webmproject.org/group/codec-devel/?hl=en.


Aron Rosenberg

unread,
Apr 8, 2011, 12:32:59 PM4/8/11
to codec...@webmproject.org
Using the same variable names is fine with us.


Aron Rosenberg
Sr. Director, Engineering
Logitech Inc. (SightSpeed Group)



Mikhal Shemer

unread,
Apr 8, 2011, 12:57:13 PM4/8/11
to codec...@webmproject.org, Aron Rosenberg
I agree. Keeping the existing names makes sense. 

-Mikhal

John Koleszar

unread,
Apr 11, 2011, 11:40:56 AM4/11/11
to codec...@webmproject.org, Mikhal Shemer, Aron Rosenberg
Ok. Updated patch available here:

https://review.webmproject.org/2142

I've given it some cursory testing, but I refactored a few things from
the original patch so it could use some more. Also, anyone have better
language for vpx_encoder.h? I still find it confusing.

tom_h...@logitech.com

unread,
Apr 11, 2011, 12:01:48 PM4/11/11
to codec...@webmproject.org

In real time mode, the higher value will force the codec to adapt to the target data rate faster. That is the essence of the patch. I haven't tested two pass mode, perhaps slow adaptation is handled in a different manner in two pass? I haven't analyzed/tested that pathway, but I would imagine the effect would be similar- less short term variation in rate (more CBR).

Tom

Inactive hide details for John Koleszar ---04/11/2011 08:48:45 AM---Ok. Updated patch available here: https://review.webmprojecJohn Koleszar ---04/11/2011 08:48:45 AM---Ok. Updated patch available here: https://review.webmproject.org/2142

John Koleszar

unread,
Apr 13, 2011, 4:05:22 PM4/13/11
to codec...@webmproject.org, tom_h...@logitech.com, Mikhal Shemer, Aron Rosenberg
Ok. I'm still not wild about the current language describing these controls, but I think we can take that as an open documentation issue and move on. Can you guys elaborate on how you're using these controls? Is this just set it and forget it (if so, to what?) or are you actually adapting this to current conditions? I'd be interested to hear about your experiences.

The next set of controls up for discussion is:
   rc_min_intra_pct;
   rc_min_inter_pct;
   rc_max_intra_pct;
   rc_initial_key_quantizer;

My thoughts:

1) Regarding rc_{min,max}_intra_pct, interesting that people have found the I-frame boost to be both too high and too low. Maybe you can share some of your experiences? 

Internally, the boost used is a curve based on the selected q. These controls don't take the selected q into account, they just apply a clamp. So at low q, rc_min_intra_pct may not have any effect if you get limited by the internal curve. Is this the behavior users want?  Or do we maybe want a multiplier or shift on the curve instead?

This control sort of makes me nervous since it's one you can shoot yourself in the foot with it pretty easily, and there are times when aggressive boosting isn't the right thing to do (like forced vs natural keyframes), which opens up issues like when do we ignore what the user's asking for, etc.

2) Regarding rc_min_inter_pct, what's the use case here? What's the behavior you're seeing? 

3) Regarding the rc_initial_key_quantizer, I think rc_max_intra_pct should cover the same use case and is more flexible in general. Suggest we proceed in that direction.

Marco Paniconi

unread,
Apr 13, 2011, 5:01:00 PM4/13/11
to codec...@webmproject.org, tom_h...@logitech.com, Mikhal Shemer, Aron Rosenberg
I would like to see at least the rc_max_intra_pct parameter exposed. 

The rc_max_intra_pct (or rc_initial_key_quantizer) is needed to at least have some control on the initial key frame size. 
The initial size of key frame is arbitrarity set to 0.5*target_bandwidth (or 3/2*target_bandwidth for error_resilience==1), too large in cases for small buffer sizes/low delay applications.

I agree with your point 3: rc_max_intra_pct could be used in place of rc_initial_key_quantizer, and then also be a cap for any key frames in sequence.

Regarding the size of subsequent key frames; note that in the function vp8_calc_iframe_target_size,
the target size for error_resilience==1 is arbitrarily fixed to 2*per_frame_bandwidth, which overrides the boost settings, and may be too small for key frame in some cases.
So rc_min_intra_pct could be used to avoid this. But not sure if vp8_calc_iframe_target_size is currently being used (it was used in previous version for fixed key frame spacing mode).

Not sure about the rc_min_inter_pct: 
The minimum size of target inter frame seems to be to  set as 1/4 * per_frame_bandwidth in one-pass mode ( in function vp8_calc_pframe_target_size(VP8_COMP *cpi)
This parameter could be used to allow the target size to go lower if needed, again for tight buffer control/low delay applications.

tom_h...@logitech.com

unread,
Apr 19, 2011, 5:02:49 PM4/19/11
to John Koleszar, Aron Rosenberg, codec...@webmproject.org, Mikhal Shemer

I am not strongly tied to using rc_min_intra_pct, but I do use it. I am fine with some other method of controlling key frame size, as long as its consistent and we don't break the error_resilient_mode case.

I use rc_min_intra_pct to make sure that this_frame_target is set to a number that should in theory use up the amount of a second's worth of bandwidth that we would prefer- in this case we are hoping that each frame doesn't consume more than say 4.5 frames of equivalent inter frame data rate. Unfortunately, I also have to use error_resilient_mode for reasons I think having to do with sending the loop filter deltas. Using error_resilient_mode makes it so any value set by rc_min_intra_pct is modified in pick_frame_size on the very first frame, and ignored in vp8_calc_iframe_target_size on subsequent key frames. I am unsure as why error_resilient_mode requires 2-3x more data, it seems like less would be needed.

So, I am fine with some other method as long as error_resilient_mode doesn't break.

As far as rc_min_inter_pct, we set it close to 0 in order to hint to the codec that we'll allow the target frame size to approach 0 in some cases in order to keep the rate constant. That we need.

Tom

Inactive hide details for John Koleszar ---04/13/2011 01:05:34 PM---Ok. I'm still not wild about the current language describinJohn Koleszar ---04/13/2011 01:05:34 PM---Ok. I'm still not wild about the current language describing these controls, but I think we can take



From: John Koleszar <jkol...@google.com>
To: codec...@webmproject.org
Cc: tom_h...@logitech.com, Mikhal Shemer <mik...@google.com>, Aron Rosenberg <arose...@logitech.com>
Date: 04/13/2011 01:05 PM
Subject: Re: [RFC] Issues with undershoot and overshoot and intra/inter frame percentages





Ok. I'm still not wild about the current language describing these controls, but I think we can take that as an open documentation issue and move on. Can you guys elaborate on how you're using these controls? Is this just set it and forget it (if so, to what?) or are you actually adapting this to current conditions? I'd be interested to hear about your experiences.

The next set of controls up for discussion is:
   rc_min_intra_pct;
   rc_min_inter_pct;
   rc_max_intra_pct;
   rc_initial_key_quantizer;

My thoughts:

1) Regarding rc_{min,max}_intra_pct, interesting that people have found the I-frame boost to be both too high and too low. Maybe you can share some of your experiences? 

Internally, the boost used is a curve based on the selected q. These controls don't take the selected q into account, they just apply a clamp. So at low q, rc_min_intra_pct may not have any effect if you get limited by the internal curve. Is this the behavior users want?  Or do we maybe want a multiplier or shift on the curve instead?

This control sort of makes me nervous since it's one you can shoot yourself in the foot with it pretty easily, and there are times when aggressive boosting isn't the right thing to do (like forced vs natural keyframes), which opens up issues like when do we ignore what the user's asking for, etc.

2) Regarding rc_min_inter_pct, what's the use case here? What's the behavior you're seeing? 

3) Regarding the rc_initial_key_quantizer, I think rc_max_intra_pct should cover the same use case and is more flexible in general. Suggest we proceed in that direction.



On Mon, Apr 11, 2011 at 12:01 PM, <tom_h...@logitech.com> wrote:

    In real time mode, the higher value will force the codec to adapt to the target data rate faster. That is the essence of the patch. I haven't tested two pass mode, perhaps slow adaptation is handled in a different manner in two pass? I haven't analyzed/tested that pathway, but I would imagine the effect would be similar- less short term variation in rate (more CBR).

    Tom


John Koleszar

unread,
Apr 21, 2011, 3:18:11 PM4/21/11
to tom_h...@logitech.com, Aron Rosenberg, codec...@webmproject.org, Mikhal Shemer
For your use of rc_min_intra_pct, it sounds like we could do a better job of detecting in the codec when P-frames are likely to be on average very small and boost the I-frame rate accordingly, rather than exposing an explicit control for it. I'd think the codec would be in a better position to make this decision than the application, because it has more context of the actual video data. Even for a VC application, the "right" value to use is going to depend on the material -- for a longer focal distance, I'd expect more of the background to be static and thus a higher I-rate to P-rate ratio, compared to close focus. If this value is set too high, you're going to get noticeable keyframe popping, which is worse for perceived quality than the selected rate being too low.

For rc_max_intra_pct, the use case identified so far is for limiting the size of the first keyframe. I had guessed (incorrectly?) that the main concern here was startup lag, so I proposed this patch[1] which limited the frame size based on the starting buffer level. With respect to my above discussion of I/P size ratios, I can see that this doesn't really expose anything that would effectively let you signal the relative size like you'd get by exposing the rate directly. But is the application really any more informed here than the codec is? ie, if we move from a constant inside the application (say 50% of the starting buffer level) to one outside the application (constant in that the app always uses the same value, but maybe different than the codec's default), have we gained anything? My concern about exposing this control to all frames is that if it's set too low, you'll get keyframe popping and "quality fade in." Are there any other uses besides the first frame that this control would be useful for? If not, maybe there's a case to be made for a control limited to the initial frame after all. Can you share some more about how you'd set it, under what circumstances you'd set it differently, etc? 


rc_min_inter_pct is an interesting one, in that I would have expected it to be 0 already, and I see after reading your patch it's 25% right now. I've been looking a lot at the buffering behavior lately and I'm seeing that it's consistently overshooting the target rate by over 25%. I wonder if it's being propped up by this limit, I'll have to test that. See this graph[2], which shows the behavior for our standard 30 fps cif test set, using 500kbps and a 500ms buffer. It shows the peak bitrate over a window of N frames, and at N=15 (500ms) the mean is 650kbps. Command line I used is:

$ ./vpxenc  --psnr $f  --target-bitrate=500 -o - --ivf --rt --end-usage=cbr  --undershoot-pct=100 --buf-sz=500 --buf-initial-sz=250 --buf-optimal-sz=250

So what's the Right behavior here? It seems to me that the only reason you'd want to set a minimum is to prevent the quality from degrading. Set the value too high and you'll never hit your bitrate. I wonder if it'd be better to have this always set to 0, and then the equivalent of rc_min_inter_pct might come up when we start talking about frame dropping? Or maybe there's a threshold below which we transmit a frame, but don't update the reference frame? 

tom harper

unread,
Apr 21, 2011, 4:52:11 PM4/21/11
to John Koleszar, tom_h...@logitech.com, Aron Rosenberg, codec...@webmproject.org, Mikhal Shemer
I think maybe I wasn't clear enough in what I was saying re rc_min_intra_pct.  I am specifically using rc_min_intra_pct to control the exact i-frame target size as a percentage of the per-second rate, not to control the rate of the issuance of i-frames.  The variable name rc_min_intra_pct is really poorly chosen, it should really just be called rc_intra_pct.  It sets how much of the target bandwidth per second is set into cpi->this_frame_target.  After that codec internal rate control buffer model takes over, as it should.  An example of this is lets say I think an i-frame should be allowed to take exactly 5/30 of the per second available rate, no more or less.  The current buffering mechanism will take note of this target and allocate bits appropriately.

I understand if you constrain the key frame size, key frame "popping" can occur but I think you should let the relative proportional i-frame size in terms of the data rate be an app level decision.

The rc_min_inter_pct change was indeed needed to prevent the rate propping problem that you are seeing, where we'd never adapt to the rate in some cases.  I am fine with setting this close to 0, i have it set at 1.  The default is set so as not to break the current behavior.

Hope this is more clear,
Tom

John Koleszar

unread,
Apr 21, 2011, 8:51:35 PM4/21/11
to tha...@logitech.com, tom harper, tom_h...@logitech.com, Aron Rosenberg, codec...@webmproject.org, Mikhal Shemer
Ok. I did understand your meaning correctly, with the exception of min vs exact, but that's not particularly important.

I understand that this is something that libvpx isn't doing a good job of right now, and I don't have a problem putting in a control for it as a workaround until we get it fixed but I'm still viewing it as a bug fix, not a new feature. I'm just not understanding why this should be an app level control, given a well behaved encoder. What information does the application have in this case that the codec doesn't? Particularly in the case of VC where many/most key frames are forced, I'd think the codec should be able to do a good job of detecting when it's over/underspent on a keyframe.. for example, if you undersize a kf, you'll spend bits bringing the background up in quality, which is going to be noticable visually, but you should be able to detect that in the codec and increase the intra size so it doesn't happen on the next kf. I'm starting to spend most of my development time on one pass encoding and rate control, so I'm most interested in understanding and fixing the underlying problems.

John

Mikhal Shemer

unread,
Apr 21, 2011, 9:22:17 PM4/21/11
to John Koleszar, tha...@logitech.com, tom harper, tom_h...@logitech.com, Aron Rosenberg, codec...@webmproject.org
I think it is agreed that the current settings introduce an initial key frame which is too big.  Currently the target size is determined as a constant factor of the target rate. The target resolution for subsequent frames is also determined by the codec state. 

Factors which may affect the target size  are: size of the frame, complexity, packet loss level etc. The current codec is not aware of the last two. One use case would be the following: When you loose a packet in the first I-frame, subsequent blocks of the frame are corrupted. Therefore, it might be beneficial to reduce the size of the first I-frame in high packet loss scenarios.    

I don't think that undersizing the key-frame by the application is a relevant concern if the application keeps the numbers reasonable. You can never be fully dummy-proof. 

-Mikhal  

John Koleszar

unread,
Apr 21, 2011, 9:49:16 PM4/21/11
to Mikhal Shemer, tha...@logitech.com, tom harper, tom_h...@logitech.com, Aron Rosenberg, codec...@webmproject.org
Thanks. Having a more concrete example like behavior in the presence of packet loss helps me understand. With respect to complexity, that's something I hope to be adding analysis for in the next few months.

I wonder if it might be more useful to describe as rc_kf_rate_bias, which would let the encoder choose whatever size it thought was appropriate, and then apply the app's specified bias on top of that, rather than letting an app set the rate directly. This would make it easy to do something like FEC, where you know you're going to add a certain overhead.. Same concept also could apply to frames that update one of the long term references.

tom harper

unread,
Apr 22, 2011, 12:33:17 AM4/22/11
to John Koleszar, Mikhal Shemer, Aron Rosenberg, codec...@webmproject.org
I agree that overriding the i-frame rate/size may just be a temporary fix.  I think we are mostly concerned, as Mikhal said, about the size of the first frame, and less concerned about subsequent i-frame requests such as those resulting from high packet loss. 

Using rc_kf_rate_bias could be good in theory, but maybe is more than is needed?  In our use case, where we employ error resilient mode, the originally proposed parameter in effect only adjusts the size of the first frame. 

I think in the short term, in terms of method, I would prefer to be able to set the rate percentage rather than have a max cap, or quantizer, as the encoder can decide what quant is appropriate.  Whatever the control is, it shouldn't need to be adjusted every time the bit rate changes. 

I think in terms of fixing the behavior longer term, one way could be to have CBR/VBR modes, or strict buffer model vs relaxed.  There could be reasonable buffer defaults and VBR behavior (with larger i frames) as default, and CBR could be set for specific use cases by the application.  These settings would adjust how the buffer is interpreted internally from how it is done currently.  I can think of several situations where you would want the more strict behavior, very high packet loss would be one of those I suppose.

Tom

Mikhal Shemer

unread,
Apr 22, 2011, 1:35:39 PM4/22/11
to tha...@logitech.com, John Koleszar, Aron Rosenberg, codec...@webmproject.org
I agree with Tom. It makes sense to set the max key size as a percentage of the target rate, as we discussed in previous threads. A reasonable solution, in my opinion, would be as follows: allow the application to set a max value on the size of the key frame as a percentage of the target rate. This value would cap the value set by the codec, i.e. would only be used if the codec decides on a higher target rate.

-Mikhal  

John Koleszar

unread,
Apr 25, 2011, 12:41:02 PM4/25/11
to Mikhal Shemer, tha...@logitech.com, Aron Rosenberg, codec...@webmproject.org
Ok. I took a cut at this in https://review.webmproject.org/2201 , and I think the other change https://review.webmproject.org/2176 is still complementary. Did I capture the discussion correctly?

John Koleszar

unread,
Apr 27, 2011, 10:02:15 AM4/27/11
to Mikhal Shemer, tha...@logitech.com, Aron Rosenberg, codec...@webmproject.org
Ok, moving on to:

   rc_dropframe_thresh;
   rc_temporaldecimation_thresh;
   vp8e_set_bitrate()

For vp8e_set_bitrate(), I agree in spirit with this. There's been a bit of discussion of this in the issue tracker[1], let me summarize:

* There's concern that the patch changes the frame rate unconditionally
* There's some desire to set the frame rate in addition to the bitrate

To which I'll add: The new function should not take vpx_codec_enc_cfg_t as an argument, it should be limited to only the parameter(s) that are being updated.

Right now, the application does not pass frame rate to the codec. Doing so would be a significant change. I think the way we calculate frame rate in one pass could be a lot better, but I think that's orthogonal to setting the bitrate?

For the frame dropping, I agree that exposing the threshold at which frames are dropped would be valuable, and tying it to the buffer level makes sense. Controlling the temporal decimation (and similarly spatial resampling) with the buffer level is probably useless for small buffers though, since dipping very low into the buffer is going to be more common, but you don't necessarily need to take drastic measures when that happens. Maybe we need a better way to express these thresholds?

Henrik Lundin

unread,
Apr 27, 2011, 11:22:27 AM4/27/11
to codec...@webmproject.org, Mikhal Shemer, tha...@logitech.com, Aron Rosenberg
Comments for vp8e_set_bitrate(): 
(no comments for frame dropping)
  • I agree that vpx_codec_enc_cfg_t should not be an argument to set_bitrate.
  • I would still like a function that can be used to update the frame rate. Today, we initialize the frame rate through the g_timebase.num and g_timebase.den. Can we consider a separate function vp8e_set_framerate() which takes new values for num and den as inputs?
/Henrik

John Koleszar

unread,
Apr 27, 2011, 11:38:10 AM4/27/11
to codec...@webmproject.org, Mikhal Shemer, tha...@logitech.com, Aron Rosenberg
Updating the time base and updating the frame rate aren't the same thing. I might argue that you shouldn't be allowed to change the time base after it's been set, since it's just going to lead to discontinuities if you don't properly rescale the time stamps you're passing in. The way this was intended to work was that you'd set a timebase that was the lowest common denominator of the frame rates you wanted to use, and then adjust the frame duration to tell the codec about the frame rate. So if you set a time base of 1/30, you could get frame rates of 30 fps, 15fps, 10fps, 7.5fps, by passing a duration of 1-4 respectively. Do you still need an additional frame rate control, given that you can effectively change the frame rate on a per-frame basis already? (I know that the actual implementation of this today is sloppy, but should be pretty easy to fix up.)

tom harper

unread,
Apr 27, 2011, 12:42:26 PM4/27/11
to John Koleszar, codec...@webmproject.org, Mikhal Shemer, Aron Rosenberg
I would prefer the timebase or frame rate not change when I change bit rate, but I could see how some folks might find it useful if it were to scale the bit rate based on the time base.   I would prefer that the basic function to set the bit rate not fiddle with the frame rate or time base.   I adjusted it in my original patch out of concern that it might break something if I didn't do it, but after further review I think it is better not to touch it unless absolutely needed.  I am fine with removing  vpx_codec_enc_cfg_t from set bit rate and just passing in the rate.

Tom

Mikhal Shemer

unread,
Apr 27, 2011, 12:55:43 PM4/27/11
to John Koleszar, codec...@webmproject.org, tha...@logitech.com, Aron Rosenberg
It is my understanding, that the frame rate in vp8 is computed every frame based on current frame's duration and the last computed frame rate. Therefore, when we update the frame rate during runtime, it takes the encoder several frames to reach the true frame rate. In addition, in current implementation, the update of the bit rate is done via an update of the cfg structure, which in turn updates the time base. Therefore, if we don't want the frame rate to reset to the timebase value on every bit rate update, the timebase should be updated as the frame rate changes. As it seems like you're doing some spring cleaning, adding a clear frame rate parameter sounds like a good idea.  

As to the frame dropper: I would like to see two thresholds, one for an actual frame dropper (based on the buffer level), and another for the temporal decimation. The current parameter (rc_dropframe_th) could be used for the first. 

-Mikhal
Reply all
Reply to author
Forward
0 new messages