Closed Bug 1366201 Opened 7 years ago Closed 7 years ago

Resync ffvpx decoder to FFmpeg 3.4

Categories

(Core :: Audio/Video: Playback, enhancement, P3)

Product:
Core
Component:
Audio/Video: Playback
Type:
enhancement

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla58
Tracking Flags:
Tracking Status
firefox58 --- fixed

People

(Reporter: jya, Assigned: jya)

References

Details

Attachments

(5 files)

Bug 1366201 - P5. Get around FFmpeg bug with corrupted data.
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
Bug 1366201 - P1. Revert "Bug 1274256 - TSan: data race gfx/layers/client/TextureClient.cpp:1161 in mozilla::layers::MappedYCbCrChannelData::CopyInto."
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
Bug 1366201 - P2. Revert "Bug 1292039: [ffmpeg] P1. Remove unused options and dictionary features."
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
Bug 1366201 - P3. Update ffvp9/ffvp8 to 3.4 branch.
59 bytes, text/x-review-board-request
mozbugz
: review+
Details
Bug 1366201 - P4. Remove no longer necessary files.
59 bytes, text/x-review-board-request
mozbugz
: review+
Details 
FFmpeg current release is 3.3.
FFmpeg 3.4 has multi-threaded slice decoding which would greatly benefit the use with WebRTC
Assignee: nobody → jyavenard
Summary: Resync ffvpx decoder to FFmpeg 3.3 one → Resync ffvpx decoder to FFmpeg 3.4
crashetest in bug 1180881 causes the updated ffvp8 decoder to crash.
See Also: → 1180881
Comment on attachment 8921993 [details]
Bug 1366201 - P1. Revert "Bug 1274256 - TSan: data race gfx/layers/client/TextureClient.cpp:1161 in mozilla::layers::MappedYCbCrChannelData::CopyInto."

https://reviewboard.mozilla.org/r/192974/#review198356

r+, trusting that it's just a revert.
But do you really need to revert this? ('volatile' feels worse than 'atomic_int')

::: media/ffvpx/libavcodec/pthread_frame.c:474
(Diff revision 1)
>  }
>  
>  void ff_thread_report_progress(ThreadFrame *f, int n, int field)
>  {
>      PerThreadContext *p;
> -    atomic_int *progress = f->progress ? (atomic_int*)f->progress->data : NULL;
> +    volatile int *progress = f->progress ? (int*)f->progress->data : NULL;

'volatile' is almost certainly incorrect if it's used for thread safety (compared to atomic_int), it would be interesting to see why they use it.
Attachment #8921993 - Flags: review?(gsquelart) → review+
Comment on attachment 8921994 [details]
Bug 1366201 - P2. Revert "Bug 1292039: [ffmpeg] P1. Remove unused options and dictionary features."

https://reviewboard.mozilla.org/r/192976/#review198358

r+ trusting it's only a revert.
Attachment #8921994 - Flags: review?(gsquelart) → review+
Comment on attachment 8921993 [details]
Bug 1366201 - P1. Revert "Bug 1274256 - TSan: data race gfx/layers/client/TextureClient.cpp:1161 in mozilla::layers::MappedYCbCrChannelData::CopyInto."

https://reviewboard.mozilla.org/r/192974/#review198356

> 'volatile' is almost certainly incorrect if it's used for thread safety (compared to atomic_int), it would be interesting to see why they use it.

this is the original code before our fix. FFmpeg developed their own fix, and is included in P3.
it ises stdatomics instead.
(In reply to Gerald Squelart [:gerald] from comment #9)

> 'volatile' is almost certainly incorrect if it's used for thread safety
> (compared to atomic_int), it would be interesting to see why they use it.

They don't use it for thread safety. This revert out us back to what FFMPEG 3.2 code was using. In P3 we include FFMPEG official change instead.
https://github.com/FFmpeg/FFmpeg/blob/master/libavcodec/pthread_frame.c#L560
Comment on attachment 8921995 [details]
Bug 1366201 - P3. Update ffvp9/ffvp8 to 3.4 branch.

https://reviewboard.mozilla.org/r/192978/#review198364

r+ trusting the outside update, I cannot review the whole thing!
Attachment #8921995 - Flags: review?(gsquelart) → review+
Comment on attachment 8921996 [details]
Bug 1366201 - P4. Remove no longer necessary files.

https://reviewboard.mozilla.org/r/192980/#review198366
Attachment #8921996 - Flags: review?(gsquelart) → review+
Comment on attachment 8921966 [details]
Bug 1366201 - P5. Get around FFmpeg bug with corrupted data.

https://reviewboard.mozilla.org/r/192968/#review198370
Attachment #8921966 - Flags: review?(gsquelart) → review+
Pushed by jyavenard@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6becd67f09e1
P1. Revert "Bug 1274256 - TSan: data race gfx/layers/client/TextureClient.cpp:1161 in mozilla::layers::MappedYCbCrChannelData::CopyInto." r=gerald
https://hg.mozilla.org/integration/autoland/rev/928c8f13b896
P2. Revert "Bug 1292039: [ffmpeg] P1. Remove unused options and dictionary features." r=gerald
https://hg.mozilla.org/integration/autoland/rev/d334969fa9f6
P3. Update ffvp9/ffvp8 to 3.4 branch. r=gerald
https://hg.mozilla.org/integration/autoland/rev/4e3060390bcd
P4. Remove no longer necessary files. r=gerald
https://hg.mozilla.org/integration/autoland/rev/6a0dc4602c74
P5. Get around FFmpeg bug with corrupted data. r=gerald
Blocks: 1411883
Depends on: 1414596
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: