New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 4 users
Status: Started
Owner:
Cc:
Components:
NextAction: ----
OS: ----
Pri: 3
Type: Enhancement



Sign in to add a comment
Refactor the VideoEncoder interface to accept the EncoderSettings stuct.
Project Member Reported by pbos@webrtc.org, May 30 2014 Back to list
For now ReconfigureEncoder etc. take a void* encoder parameter to be cast back when used usage. This isn't used/forwarded at the moment. The idea is that there shouldn't be a settings union, but the implementing user can set codec parameters regardless of whether webrtc/ knows the codec exists or not.
 
Project Member Comment 1 by bugdroid1@chromium.org, Jul 10 2014
The following revision refers to this bug:
  http://code.google.com/p/webrtc/source/detail?r=6650

------------------------------------------------------------------
r6650 | pbos@webrtc.org | 2014-07-10T10:13:37.529558Z

Changed paths:
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/video/video_send_stream.cc&spec=svn6650&r_previous=6649&r=6650&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/video/video_send_stream_tests.cc&spec=svn6650&r_previous=6649&r=6650&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/test/call_test.cc&spec=svn6650&r_previous=6649&r=6650&format=side
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/test/call_test.h&spec=svn6650&r_previous=6649&r=6650&format=side

Support VP8 encoder settings in VideoSendStream.

Stop-gap solution to support VP8 codec settings in the new API until
encoder settings can be passed on to the VideoEncoder without requiring
explicit support for the codec.

BUG=3424
R=stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/17929004
-----------------------------------------------------------------
Project Member Comment 2 by bugdroid1@chromium.org, Jul 10 2014
The following revision refers to this bug:
  http://code.google.com/p/webrtc/source/detail?r=6652

------------------------------------------------------------------
r6652 | pbos@webrtc.org | 2014-07-10T13:21:40.505789Z

Changed paths:
   M http://code.google.com/p/webrtc/source/diff?path=/trunk/webrtc/video/video_send_stream_tests.cc&spec=svn6652&r_previous=6651&r=6652&format=side

Skip encoding in fake VP8 encoder.

Broke memcheck, FakeEncoder::Encode doesn't produce valid VP8 frames.

BUG=3424
R=stefan@webrtc.org

Review URL: https://webrtc-codereview.appspot.com/13899004
-----------------------------------------------------------------
Project Member Comment 3 by pbos@webrtc.org, Sep 22 2014
Cc: stefan@webrtc.org
Summary: Refactor the VideoEncoder interface to accept the EncoderSettings stuct. (was: Forward encoder parameters in the new API)
Renaming this as the forwarding itself is done for the existing encoders (copied to VideoCodecVP8/VideoCodecH264), what's left is to change the VideoEncoder API so that we can forward encoder_settings codec-agnostically.
Project Member Comment 4 by mflodman@webrtc.org, Sep 30 2014
Labels: Area-Video
Project Member Comment 5 by tnakamura@webrtc.org, Nov 4 2015
This bug hasn't been modified for more than a year. Is this still a valid open issue?
Project Member Comment 6 by pbos@webrtc.org, Nov 5 2015
Yep, just a bit unlikely to happen very soon.
Project Member Comment 7 by mflodman@webrtc.org, Mar 1 2016
Labels: EngTriaged
Project Member Comment 8 by pbos@webrtc.org, Jul 18 2016
Cc: pbos@webrtc.org mflodman@webrtc.org
Owner: perkj@webrtc.org
Reassigning, I'll still own getting the void* removal in, cl here: https://codereview.webrtc.org/2047513002/
Project Member Comment 9 by bugdroid1@chromium.org, Sep 27 2016
The following revision refers to this bug:
  https://chromium.googlesource.com/external/webrtc.git/+/29a44e351e1e35c29dd5ff650f62cbc4d10b3b1b

commit 29a44e351e1e35c29dd5ff650f62cbc4d10b3b1b
Author: kthelgason <kthelgason@webrtc.org>
Date: Tue Sep 27 10:52:02 2016

This is a resubmission of https://codereview.webrtc.org/2047513002/

Original description:
Add proper lifetime of encoder-specific settings.

Permits passing VideoEncoderConfig between threads and not worry about
the lifetime of an underlying void pointer. Also adds type safety to
unpacking of codec-specific settings.

These settings are not yet propagating to VideoEncoder interfaces, but
the aim is to get rid of webrtc::VideoCodec for VideoEncoder.

BUG=webrtc:3424
R=perkj@webrtc.org, pbos@webrtc.org
TBR=mflodman@webrtc.org

Review-Url: https://codereview.webrtc.org/2347843002
Cr-Commit-Position: refs/heads/master@{#14396}

[modify] https://crrev.com/29a44e351e1e35c29dd5ff650f62cbc4d10b3b1b/webrtc/config.cc
[modify] https://crrev.com/29a44e351e1e35c29dd5ff650f62cbc4d10b3b1b/webrtc/config.h
[modify] https://crrev.com/29a44e351e1e35c29dd5ff650f62cbc4d10b3b1b/webrtc/media/engine/fakewebrtccall.cc
[modify] https://crrev.com/29a44e351e1e35c29dd5ff650f62cbc4d10b3b1b/webrtc/media/engine/webrtcvideoengine2.cc
[modify] https://crrev.com/29a44e351e1e35c29dd5ff650f62cbc4d10b3b1b/webrtc/media/engine/webrtcvideoengine2.h
[modify] https://crrev.com/29a44e351e1e35c29dd5ff650f62cbc4d10b3b1b/webrtc/modules/audio_coding/BUILD.gn
[modify] https://crrev.com/29a44e351e1e35c29dd5ff650f62cbc4d10b3b1b/webrtc/video/video_quality_test.cc
[modify] https://crrev.com/29a44e351e1e35c29dd5ff650f62cbc4d10b3b1b/webrtc/video/video_quality_test.h
[modify] https://crrev.com/29a44e351e1e35c29dd5ff650f62cbc4d10b3b1b/webrtc/video/video_send_stream_tests.cc
[modify] https://crrev.com/29a44e351e1e35c29dd5ff650f62cbc4d10b3b1b/webrtc/video/vie_encoder.cc

Project Member Comment 10 by kthelgason@webrtc.org, Sep 28 2016
Cc: perkj@webrtc.org
Owner: kthelgason@webrtc.org
Status: Fixed
Project Member Comment 11 by pbos@webrtc.org, Sep 28 2016
Status: Started
This shouldn't be fixed before webrtc::VideoEncoder::Init takes EncoderSettings instead of webrtc::VideoCodec, and webrtc::VideoCodec goes away.

I think a default implementation in webrtc::VideoEncoder::Init can convert from EncoderSettings to webrtc::VideoCodec as a first step, and webrtc::VideoCodec can be kept around until all implementations are moved over to implement the new Init interface.
Project Member Comment 12 by kthelgason@webrtc.org, Feb 10 2017
Labels: -Pri-2 Pri-3
Sign in to add a comment