Bug 232194 – Likely typo in MediaPlayerPrivateAVFoundationObjC::updateVideoTracks()

• Component: Media
• Status: RESOLVED FIXED

[Jean-Yves Avenard [:jya]]

In MediaPlayerPrivateAVFoundationObjC::updateVideoTracks() we see:

```
    for (auto& track : m_audioTracks)
        track->resetPropertiesFromTrack();

```

it's almost certainly wrong and should be:
```
    for (auto& track : m_videoTracks)
        track->resetPropertiesFromTrack();
```

[Radar WebKit Bug Importer]

<rdar://problem/84574134>

[Jean-Yves Avenard [:jya]]

Created attachment 442247 [details]
Patch

[Darin Adler]

Comment on attachment 442247 [details]
Patch

Typically when we spot an obvious mistake like this, we spend a moment to try to create a test that failed because of the error. It’s frustrating, because likely if we had written the code correctly we would not have to think about how to write a test.

Please consider if we could have tested. I have personally been in this situation many times, spotted a typo that’s an obvious mistake and then was frustrated at the challenge of writing a test to prove the code was malfunctioning, and I understand if after thinking it over you do not find it’s practical to test, but I would appreciate a specific comment saying so if that happened.

[Jean-Yves Avenard [:jya]]

(In reply to Darin Adler from comment #3)
> Comment on attachment 442247 [details]
> Patch
> 
> Typically when we spot an obvious mistake like this, we spend a moment to
> try to create a test that failed because of the error. It’s frustrating,
> because likely if we had written the code correctly we would not have to
> think about how to write a test.

I found this while reading the code for a totally unrelated bug. 

I did try to understand the consequences of such error. This code was written in 2014, so the severity of the problem is likely low. 

To hit that code, you would need to write a specially crafted HLS stream, which in itself isn’t trivial as playback of those is non deterministic. And even so, it would likely not be observable via JS. 

I will add a comment indicating so.

[Jean-Yves Avenard [:jya]]

Created attachment 442327 [details]
Patch

[EWS]

Committed r284770 (243479@main): <https://commits.webkit.org/243479@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 442327 [details].