Closed Bug 1030324 Opened 5 years ago Closed 4 years ago

VP8 Encoder generates horizontal frame distortions when re-configured for new frame size after init.

Categories

(Core :: Audio/Video, defect, P1, major)

defect

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: pkerr, Assigned: pkerr)

References

Details

Attachments

(1 file)

When the VP8 encoder receives a frame that is smaller than the initial codec configuration, setting the new frame size via the vpx_codec_enc_config_set() function causes subsequent non-keyframes to exhibit horizontal streaks in the encoded frames. Doing a full re-initialization using vpx_codec_enc_init() in its place does not trigger horizontal streaks but does cause a momentary video glitch. This is the current work around for the issue. Frame size change detection and VP8 encoder reconfiguration is handled in VP8_impl.cc::VP8EncoderImpl::Encode.
Blocks: 1027100
Priority: -- → P1
Target Milestone: --- → mozilla33
You can see examples of the distortions and a patch showing data before encode and after decode in bug 1027100.
No longer blocks: 1027100
Depends on: 1027100
Target Milestone: mozilla33 → ---
backlog: --- → webRTC+
Rank: 14
Reply to Tim's comment on the original bug this spin off of:

(In reply to Timothy B. Terriberry (:derf) from comment #18)
> (In reply to Randell Jesup [:jesup] from comment #12)
> > derf: see comment 11 (and comment 10); preliminary info may point to a
> > libvpx issue when the input size changes with a stream.
> > 
> > The corruption looks like some sort of stride issue.
> 
> I'm not aware of any libvpx issues like this. We support resolution changes
> in the <video> tag, too, and have had people file bugs about correctly
> scaling them afterwards, so use of VP8's resolution-switching in the web is
> non-zero. See bug 626979, where there's a file encoded like this attached.

This appears (without any definitive proof) to be an encoder-side issue with non-key frames after a resolution change.  So decoders handling resolution changes wouldn't be relevant.  Tim - any thoughts?

We should re-try this with the latest libvpx just to be sure it wasn't fixed.  But we should push people upstream if this can be verified.

-> libvpx, which is where this bug should have been
Component: WebRTC: Audio/Video → Video/Audio
Flags: needinfo?(tterribe)
Flags: needinfo?(giles)
backlog: webRTC+ → ---
> but does cause a momentary video glitch.

What are the properties of the "glitch"?
Flags: needinfo?(tterribe)
libvpx 1.4.0 landed in nightly on Friday, so you should be able to retest now.
Flags: needinfo?(giles)
(In reply to Timothy B. Terriberry (:derf) from comment #3)
> > but does cause a momentary video glitch.
> 
> What are the properties of the "glitch"?

IIRC a frame or 3 freeze.  Paul?  (Or I'll force it to happen to see)
Flags: needinfo?(pkerr)
The glitch was a blocky distortion smeared across the image. The edges of the distortion followed the macro-blocks.
Flags: needinfo?(pkerr)
(In reply to Paul Kerr [:pkerr] from comment #6)
> The glitch was a blocky distortion smeared across the image. The edges of
> the distortion followed the macro-blocks.

Just to verify, we're talking about the glitch that happens with the workaround (reset the encoder on resolution change), not the errors we see if we merely reconfigure it, right?
Flags: needinfo?(pkerr)
Sorry, I thought the question referred to the original distortion pattern.

From 1 to 2 frames freeze until the new resolution takes effect. A momentary glitch.
Flags: needinfo?(pkerr)
(In reply to Paul Kerr [:pkerr] from comment #8)
> From 1 to 2 frames freeze until the new resolution takes effect. A momentary
> glitch.

Okay, that sounds like something probably caused by our (where "our" very well may be webrtc.org's) logic around libvpx, rather than libvpx itself.
See Also: → 1198107
(In reply to Paul Kerr [:pkerr] from comment #6)
> The glitch was a blocky distortion smeared across the image. The edges of
> the distortion followed the macro-blocks.

I did test without your workaround (bug 1027100) in bug 1198107 and couldn't see any glitches. Though since I am not sure how you reproduced it, or what to look for exactly, I'll leave the final judgment to you Paul.
I will re-test. My original test procedure was to use the stress utility on Linux and OS X to drive the load up and cause the LoadManager to make a call to change the resolution of the video encoder. That is when the blocky distortions would appear.
I have run the tests using the stress utility again without the work around and the distortions seem to be fixed. I checked the trace log with LoadManager and webrtc_trace modules enabled and saw overload messages from LoadManager and requests to change the capture size down in the webrtc_trace VIDEO component messages. I believe that the VP8 encoder is changing capture size without the need to be re-initialized at each step down. I did not see the glitches and the resolution changed visibly during stress testing.
Ok, great - let's back out the re-init hack then.
I ran a more thorough test under the xcode debugger to verify that the VP8 encoder was receiving VP8EncoderImpl::UpdateCodecFrameSize() requests triggered by the LoadManager. With the work-around removed, the current version of VP8 does not exhibit the distortion problem. The attached patch removes the work-around.
Assignee: nobody → pkerr
Status: NEW → ASSIGNED
The VP8 encoder was re-initialized completely when the frame size changed
instead of calling the vpx_enc_config_set() method. The work around is not
longer needed.
Attachment #8657361 - Flags: review?(pehrsons)
Attachment #8657361 - Flags: review?(pehrsons) → review+
https://hg.mozilla.org/mozilla-central/rev/4912093a0f33
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.