Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2292683002:
Fix AnnexB header bytes for VideoToolbox (Closed)
DescriptionFix AnnexB header bytes for VideoToolbox
This CL addresses the bug where we see iOS HW decoder dropping most of the
frames in a H264 connection with OSX HW encoder. There is a mismatch between
the header bytes of these two implementations. iOS expect a 4 bytes header whereas
currently 3 bytes are written by OSX encoder. Making them both based on 4 bytes
fixes the dropped frames issue.
[0] https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/codecs/h264/h264_video_toolbox_nalu.cc?rcl=0&l=345
BUG=webrtc:5931
TEST= Tested H264 connection between Macbook Air and iOS.
Committed: https://crrev.com/885d82bc302b12ae7dc97db6a8c43fd5ffcf277c
Cr-Commit-Position: refs/heads/master@{#415376}
Patch Set 1 : #
Messages
Total messages: 26 (17 generated)
The CQ bit was checked by emircan@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Description was changed from ========== fix start code BUG= ========== to ========== Fix AnnexB header bytes for VideoToolbox BUG=webrtc:5931 ==========
Description was changed from ========== Fix AnnexB header bytes for VideoToolbox BUG=webrtc:5931 ========== to ========== Fix AnnexB header bytes for VideoToolbox This CL addresses the bug where we see iOS HW decoder dropping most of the frames in a H264 connection with OSX HW encoder. There is a mismatch between the header bytes of these two implementations. iOS expect a 4 byte header whereas current 3 bytes are written. Making them both based on 4 bytes fixes the dropped frames issue. [0] https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/... BUG=webrtc:5931 TEST= Testsed H264 connection between Macbook Air and iOS. ==========
emircan@chromium.org changed reviewers: + jfroy@chromium.org, tkchin@chromium.org
Description was changed from ========== Fix AnnexB header bytes for VideoToolbox This CL addresses the bug where we see iOS HW decoder dropping most of the frames in a H264 connection with OSX HW encoder. There is a mismatch between the header bytes of these two implementations. iOS expect a 4 byte header whereas current 3 bytes are written. Making them both based on 4 bytes fixes the dropped frames issue. [0] https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/... BUG=webrtc:5931 TEST= Testsed H264 connection between Macbook Air and iOS. ========== to ========== Fix AnnexB header bytes for VideoToolbox This CL addresses the bug where we see iOS HW decoder dropping most of the frames in a H264 connection with OSX HW encoder. There is a mismatch between the header bytes of these two implementations. iOS expect a 4 byte header whereas current 3 bytes are written. Making them both based on 4 bytes fixes the dropped frames issue. [0] https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/... BUG=webrtc:5931 TEST= Tested H264 connection between Macbook Air and iOS. ==========
PTAL.
The CQ bit was checked by emircan@chromium.org to run a CQ dry run
Patchset #1 (id:1) has been deleted
Note that we should also modify WebRTC standalone's H264 parser to handle both 0001 and 001 as header bytes. Currently WebRTC only expects 4-byte header[1] but Chrome handles both cases[0]. tkchin@ let me know what you think. [0] https://cs.chromium.org/chromium/src/media/filters/h264_parser.cc?rcl=1472469... [1] https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/...
Description was changed from ========== Fix AnnexB header bytes for VideoToolbox This CL addresses the bug where we see iOS HW decoder dropping most of the frames in a H264 connection with OSX HW encoder. There is a mismatch between the header bytes of these two implementations. iOS expect a 4 byte header whereas current 3 bytes are written. Making them both based on 4 bytes fixes the dropped frames issue. [0] https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/... BUG=webrtc:5931 TEST= Tested H264 connection between Macbook Air and iOS. ========== to ========== Fix AnnexB header bytes for VideoToolbox This CL addresses the bug where we see iOS HW decoder dropping most of the frames in a H264 connection with OSX HW encoder. There is a mismatch between the header bytes of these two implementations. iOS expect a 4 byte header whereas currently 3 bytes are written by OSX encoder. Making them both based on 4 bytes fixes the dropped frames issue. [0] https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/... BUG=webrtc:5931 TEST= Tested H264 connection between Macbook Air and iOS. ==========
On 2016/08/29 21:09:16, emircan wrote: > Note that we should also modify WebRTC standalone's H264 parser to handle both > 0001 and 001 as header bytes. Currently WebRTC only expects 4-byte header[1] but > Chrome handles both cases[0]. tkchin@ let me know what you think. > > [0] > https://cs.chromium.org/chromium/src/media/filters/h264_parser.cc?rcl=1472469... > [1] > https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/... lgtm previously we only handled the 4 byte case today because the webrtc rtp depacketizer always gave us the 4 byte one. This probably changed underneath and I wasn't aware of it. I think it would be good to handle both - can you file a bug on holmer@?
Description was changed from ========== Fix AnnexB header bytes for VideoToolbox This CL addresses the bug where we see iOS HW decoder dropping most of the frames in a H264 connection with OSX HW encoder. There is a mismatch between the header bytes of these two implementations. iOS expect a 4 byte header whereas currently 3 bytes are written by OSX encoder. Making them both based on 4 bytes fixes the dropped frames issue. [0] https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/... BUG=webrtc:5931 TEST= Tested H264 connection between Macbook Air and iOS. ========== to ========== Fix AnnexB header bytes for VideoToolbox This CL addresses the bug where we see iOS HW decoder dropping most of the frames in a H264 connection with OSX HW encoder. There is a mismatch between the header bytes of these two implementations. iOS expect a 4 bytes header whereas currently 3 bytes are written by OSX encoder. Making them both based on 4 bytes fixes the dropped frames issue. [0] https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/... BUG=webrtc:5931 TEST= Tested H264 connection between Macbook Air and iOS. ==========
On 2016/08/29 21:15:53, tkchin wrote: > On 2016/08/29 21:09:16, emircan wrote: > > Note that we should also modify WebRTC standalone's H264 parser to handle both > > 0001 and 001 as header bytes. Currently WebRTC only expects 4-byte header[1] > but > > Chrome handles both cases[0]. tkchin@ let me know what you think. > > > > [0] > > > https://cs.chromium.org/chromium/src/media/filters/h264_parser.cc?rcl=1472469... > > [1] > > > https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/... > > lgtm > > previously we only handled the 4 byte case today because the webrtc rtp > depacketizer always gave us the 4 byte one. This probably changed underneath and > I wasn't aware of it. I think it would be good to handle both - can you file a > bug on holmer@? Thanks. I filed https://bugs.chromium.org/p/webrtc/issues/detail?id=6278.
emircan@chromium.org changed reviewers: + dalecurtis@chromium.org
dalecurtis@, can you PTAL as RS OWNERS?
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
lgtm
The CQ bit was checked by emircan@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Fix AnnexB header bytes for VideoToolbox This CL addresses the bug where we see iOS HW decoder dropping most of the frames in a H264 connection with OSX HW encoder. There is a mismatch between the header bytes of these two implementations. iOS expect a 4 bytes header whereas currently 3 bytes are written by OSX encoder. Making them both based on 4 bytes fixes the dropped frames issue. [0] https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/... BUG=webrtc:5931 TEST= Tested H264 connection between Macbook Air and iOS. ========== to ========== Fix AnnexB header bytes for VideoToolbox This CL addresses the bug where we see iOS HW decoder dropping most of the frames in a H264 connection with OSX HW encoder. There is a mismatch between the header bytes of these two implementations. iOS expect a 4 bytes header whereas currently 3 bytes are written by OSX encoder. Making them both based on 4 bytes fixes the dropped frames issue. [0] https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/... BUG=webrtc:5931 TEST= Tested H264 connection between Macbook Air and iOS. ==========
Message was sent while issue was closed.
Committed patchset #1 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Fix AnnexB header bytes for VideoToolbox This CL addresses the bug where we see iOS HW decoder dropping most of the frames in a H264 connection with OSX HW encoder. There is a mismatch between the header bytes of these two implementations. iOS expect a 4 bytes header whereas currently 3 bytes are written by OSX encoder. Making them both based on 4 bytes fixes the dropped frames issue. [0] https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/... BUG=webrtc:5931 TEST= Tested H264 connection between Macbook Air and iOS. ========== to ========== Fix AnnexB header bytes for VideoToolbox This CL addresses the bug where we see iOS HW decoder dropping most of the frames in a H264 connection with OSX HW encoder. There is a mismatch between the header bytes of these two implementations. iOS expect a 4 bytes header whereas currently 3 bytes are written by OSX encoder. Making them both based on 4 bytes fixes the dropped frames issue. [0] https://cs.chromium.org/chromium/src/third_party/webrtc/modules/video_coding/... BUG=webrtc:5931 TEST= Tested H264 connection between Macbook Air and iOS. Committed: https://crrev.com/885d82bc302b12ae7dc97db6a8c43fd5ffcf277c Cr-Commit-Position: refs/heads/master@{#415376} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/885d82bc302b12ae7dc97db6a8c43fd5ffcf277c Cr-Commit-Position: refs/heads/master@{#415376} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
