Closed Bug 1137474 Opened 5 years ago Closed 5 years ago

VP9 support in signaling and conduit/pipeline code

Categories

(Core :: WebRTC: Signaling, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: jesup, Assigned: jesup)

References

(Blocks 2 open bugs)

Details

Attachments

(2 files, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #1090742 +++

This bug covers the upstream (webrtc.org) basic portions of vp9 support; it also doesn't cover things about *when* VP9 should be preferred or switched to/from (which may require allowing accepting multiple codecs, which we don't do yet)

Note that webrtc branch 40 has vp9 codec support, but it's incomplete and does not include any packetization, and many codepaths don't account for vp9 as a possibility.
Attachment #8570158 - Attachment is obsolete: true
Attachment #8570159 - Flags: review?(pkerr)
Comment on attachment 8570712 [details] [diff] [review]
Basic vp9 support added to upstream (using 'generic' packetization)

Note: this is known-to-be incomplete, as upstream isn't ready (no VP9 packetization, etc), and I may well back this out before applying the next pull from upstream, then see what they missed (assuming they get to this before the next time we pull).
Attachment #8570712 - Flags: review?(pkerr)
Attachment #8570159 - Flags: review?(pkerr) → review+
Comment on attachment 8570712 [details] [diff] [review]
Basic vp9 support added to upstream (using 'generic' packetization)

Review of attachment 8570712 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good to me. Left some minor questions.

::: media/webrtc/trunk/webrtc/modules/video_coding/main/source/video_sender.cc
@@ +136,5 @@
>                    << sendCodec->plName << ". Error code: " << ret;
>      return VCM_CODEC_ERROR;
>    }
>  
> +  // XXX fix VP9

Is there a bug tracking this fix?

::: media/webrtc/trunk/webrtc/video_engine/vie_codec_impl.cc
@@ +87,5 @@
>                   << codec.codecSpecific.H264.spsLen
>                   << ", ppslen: "
>                   << codec.codecSpecific.H264.ppsLen;
> +  } else if (codec.codecType == kVideoCodecVP9) {
> +    LOG(LS_INFO) << "VP9 specific settings";

I think it would be helpful to have some basic information here, as with the VP8 codec above.

::: media/webrtc/trunk/webrtc/video_engine/vie_encoder.cc
@@ +635,5 @@
>                         &codec_specific_info);
>      return;
>    }
>  #endif
> +  // XXX fix VP9

Is there a bug for this fix?
Attachment #8570712 - Flags: review?(pkerr) → review+
Blocks: 1138629
> ::: media/webrtc/trunk/webrtc/video_engine/vie_codec_impl.cc
> @@ +87,5 @@
> >                   << codec.codecSpecific.H264.spsLen
> >                   << ", ppslen: "
> >                   << codec.codecSpecific.H264.ppsLen;
> > +  } else if (codec.codecType == kVideoCodecVP9) {
> > +    LOG(LS_INFO) << "VP9 specific settings";
> 
> I think it would be helpful to have some basic information here, as with the
> VP8 codec above.

None of that info is filled in currently, given the lack of packetization data.  It's largely a placeholder
You need to log in before you can comment on or make changes to this bug.