1316205 - Remove legacy WaveReader

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Jean-Yves Avenard [:jya]]

HAs been unused for over a year now...

Comment 1

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1316205: P1. Remove unused WaveReader.

Review commit: https://reviewboard.mozilla.org/r/91606/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/91606/

Comment 2

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1316205: P2. Remove wave duplicated code.

Review commit: https://reviewboard.mozilla.org/r/91608/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/91608/

Comment 3

[Tzuhao Kuo [:kaku]]

Comment on attachment 8808889 [details]
Bug 1316205: P1. Remove unused WaveReader.

https://reviewboard.mozilla.org/r/91606/#review91528

Comment 4

[Tzuhao Kuo [:kaku]]

Comment on attachment 8808890 [details]
Bug 1316205: P2. Remove wave duplicated code.

https://reviewboard.mozilla.org/r/91608/#review91524

::: dom/media/DecoderTraits.cpp:210
(Diff revision 1)
>        // We can only reach this position if a particular codec was requested,
>        // ogg is supported and working: the codec must be invalid.
>        return CANPLAY_NO;
>      }
>    }
> -  if (IsWaveType(aType.GetMIMEType())) {
> +  if (IsWaveSupportedType(aType.GetMIMEType())) {

Personally, I think a wrapper function, likes the other formats do, helps to read this nested if-statement.

```
static bool 
IsWaveTypeAndEnabled(const nsACString& aType)
{
  return IsWaveSupportedType(aType);
}
```

Comment 5

[Jean-Yves Avenard [:jya]]

Comment on attachment 8808890 [details]
Bug 1316205: P2. Remove wave duplicated code.

https://reviewboard.mozilla.org/r/91608/#review91524

> Personally, I think a wrapper function, likes the other formats do, helps to read this nested if-statement.
> 
> ```
> static bool 
> IsWaveTypeAndEnabled(const nsACString& aType)
> {
>   return IsWaveSupportedType(aType);
> }
> ```

Those "wrappers" are only defined due to 2 members being public and the wish to minimize code duplication. the majority don't use a wrapper.
As it is now, it's using the same style as the line just above with IsOggSupportedType

Comment 6

[Jean-Yves Avenard [:jya]]

Attachment: Bug 1316205: P3. Remove WaveDecoder::IsEnable().

It would always return true, and there's already a MediaDecoder::IsWaveEnabled()

Review commit: https://reviewboard.mozilla.org/r/91820/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/91820/

Comment 7

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Comment on attachment 8809232 [details]
Bug 1316205: P3. Remove WaveDecoder::IsEnable().

https://reviewboard.mozilla.org/r/91820/#review91796

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c61363a3c271
P1. Remove unused WaveReader. r=kaku
https://hg.mozilla.org/integration/autoland/rev/133477f27fe6
P2. Remove wave duplicated code. r=kaku
https://hg.mozilla.org/integration/autoland/rev/60bf1293eabf
P3. Remove WaveDecoder::IsEnable(). r=gerald

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/c61363a3c271
https://hg.mozilla.org/mozilla-central/rev/133477f27fe6
https://hg.mozilla.org/mozilla-central/rev/60bf1293eabf