[PATCH v2 0/1] MR7439: winedmo: Find mp3 stream info if channel data is missing.

[Brendan McGrath (＠redmcg)]

This fixes missing mp3 channel data in the FFmpeg demuxer.

--
  v2: winedmo: Find mp3 stream info if channel data is missing.
 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7439

[PATCH v2 1/1] winedmo: Find mp3 stream info if channel data is missing.

[Brendan McGrath]

From: Brendan McGrath <bmcgrath(a)codeweavers.com>

---
 dlls/winedmo/unix_demuxer.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/dlls/winedmo/unix_demuxer.c b/dlls/winedmo/unix_demuxer.c
index 15d906e0c5f..5e22aaf9a81 100644
--- a/dlls/winedmo/unix_demuxer.c
+++ b/dlls/winedmo/unix_demuxer.c
@@ -124,6 +124,17 @@ static NTSTATUS demuxer_create_streams( struct demuxer *demuxer )
                 continue;
             }
         }
+        else if (par->codec_id == AV_CODEC_ID_MP3)
+        {
+            int channels;
+#if LIBAVUTIL_VERSION_MAJOR >= 58
+            channels = demuxer->ctx->streams[i]->codecpar->ch_layout.nb_channels;
+#else
+            channels = demuxer->ctx->streams[i]->codecpar->channels;
+#endif
+            if (!channels)
+                avformat_find_stream_info(demuxer->ctx, NULL);
+        }
 
         av_bsf_get_null_filter( &stream->filter );
         avcodec_parameters_copy( stream->filter->par_in, demuxer->ctx->streams[i]->codecpar );
-- 
GitLab

https://gitlab.winehq.org/wine/wine/-/merge_requests/7439

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winedmo/unix_demuxer.c:
> continue;
>              }
>          }
> +        else if (par->codec_id == AV_CODEC_ID_MP3)
> +        {
> +            int channels;
> +#if LIBAVUTIL_VERSION_MAJOR >= 58
> +            channels = demuxer->ctx->streams[i]->codecpar->ch_layout.nb_channels;
> +#else
> +            channels = demuxer->ctx->streams[i]->codecpar->channels;
> +#endif
> +            if (!channels)
> +                avformat_find_stream_info(demuxer->ctx, NULL);
> +        }

```suggestion:-10+0
        else if (par->codec_id == AV_CODEC_ID_MP3)
        {
            int channels;
#if LIBAVUTIL_VERSION_MAJOR >= 58
            channels = par->ch_layout.nb_channels;
#else
            channels = par->channels;
#endif
            if (!channels) avformat_find_stream_info( demuxer->ctx, NULL );
        }
```

But I wonder if this is this the best way to do this. Maybe we should call `avformat_find_stream_info` unconditionally for some formats. It was avoided because it can take a bit more time and we want demuxer initialization to be fastest as possible (to match native and address some Unreal Engine potential race condition), but that's maybe only specific to MP4 as far as I know.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7439#note_96197

[Brendan McGrath (＠redmcg)]

On Fri Feb 28 07:40:35 2025 +0000, Rémi Bernon wrote:
> ```suggestion:-10+0
>         else if (par->codec_id == AV_CODEC_ID_MP3)
>         {
>             int channels;
> #if LIBAVUTIL_VERSION_MAJOR >= 58
>             channels = par->ch_layout.nb_channels;
> #else
>             channels = par->channels;
> #endif
>             if (!channels) avformat_find_stream_info( demuxer->ctx, NULL );
>         }
> ```
> But I wonder if this is this the best way to do this. Maybe we should
> call `avformat_find_stream_info` unconditionally for some formats. It
> was avoided because it can take a bit more time and we want demuxer
> initialization to be fastest as possible (to match native and address
> some Unreal Engine potential race condition), but that's maybe only
> specific to MP4 as far as I know.

So something like:
```suggestion:-0+0
        }

        if (!(av_codec_get_tag(demuxer->ctx->iformat->codec_tag, AV_CODEC_ID_MPEG4)))
        {
            avformat_find_stream_info(demuxer->ctx, NULL);
        }
```
?

Although it would probably make more sense to do this in `demuxer_create` after the call to `avformat_open_input` but before the call to `demuxer_create_streams`.

But I'm not that familiar with the FFmpeg API (which is why I narrowed the scope of the change), so I'm happy to take your lead on this.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7439#note_96274

[Rémi Bernon]

Rémi Bernon (@rbernon) commented about dlls/winedmo/unix_demuxer.c:
> }
>      format = demuxer->ctx->iformat;
>
> if ((params->duration = get_context_duration( demuxer->ctx )) == AV_NOPTS_VALUE)

```suggestion:-0+0
    if ((params->duration = get_context_duration( demuxer->ctx )) == AV_NOPTS_VALUE ||
        strstr( format->name, "mp3" ))
```

Something like that, or whatever is best fit for what you want to fix. I think we should keep it opt-in, unless there's reason not to.

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7439#note_96511

[Brendan McGrath (＠redmcg)]

On Mon Mar  3 08:59:27 2025 +0000, Rémi Bernon wrote:
> ```suggestion:-0+0
>     if ((params->duration = get_context_duration( demuxer->ctx )) ==
> AV_NOPTS_VALUE ||
>         strstr( format->name, "mp3" ))
> ```
> Something like that, or whatever is best fit for what you want to fix. I
> think we should keep it opt-in, unless there's reason not to.

OK, I got it. That looks good. Thanks @rbernon

-- 
https://gitlab.winehq.org/wine/wine/-/merge_requests/7439#note_96513