Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metadata should be added after container conversion #5594

Closed
fstirlitz opened this issue May 3, 2015 · 7 comments
Closed

Metadata should be added after container conversion #5594

fstirlitz opened this issue May 3, 2015 · 7 comments
Labels

Comments

@fstirlitz
Copy link
Contributor

@fstirlitz fstirlitz commented May 3, 2015

$ youtube-dl -f worst b4NDjo3CuvU --recode-video mkv --add-metadata -o 'B.%(ext)s'
[youtube] b4NDjo3CuvU: Downloading webpage
[youtube] b4NDjo3CuvU: Extracting video information
[youtube] b4NDjo3CuvU: Downloading DASH manifest
[download] Destination: B.3gp
[download] 100% of 873.60KiB in 00:03
[ffmpeg] Adding metadata to 'B.3gp'
[ffmpeg] Converting video from 3gp to mkv, Destination: B.mkv
Deleting original file B.3gp (pass -k to keep)
$ ffprobe B.mkv 
[…]
Input #0, matroska,webm, from 'B.mkv':
  Metadata:
    COMPATIBLE_BRANDS: isomiso23gp4
    MAJOR_BRAND     : 3gp4
    MINOR_VERSION   : 512
    ENCODER         : Lavf56.31.100
  Duration: 00:02:58.12, start: 0.155000, bitrate: 43 kb/s
    Stream #0:0(und): Video: h264 (High), yuv420p, 176x144 [SAR 1:1 DAR 11:9], 12 fps, 12 tbr, 1k tbn, 24 tbc (default)
    Metadata:
      LANGUAGE        : und
      HANDLER_NAME    : VideoHandler
      ENCODER         : Lavc56.35.101 libx264
    Stream #0:1(eng): Audio: vorbis, 22050 Hz, mono, fltp (default)
    Metadata:
      LANGUAGE        : eng
      HANDLER_NAME    : SoundHandler
      ENCODER         : Lavc56.35.101 libvorbis 

Because ffmpeg cannot add metadata to the 3gp file, no metadata ends up in the resulting mkv file. Metadata should be added after the video is converted.

On a related note, there seems to be no option to put video in a different container without re-encoding it (i.e. -c copy).

@jaimeMF
Copy link
Collaborator

@jaimeMF jaimeMF commented May 3, 2015

The reason why we add it first is that (If I recall correctly) ffmpeg copies the metadata, which can be useful if you use --keep-video.

On a related note, there seems to be no option to put video in a different container without re-encoding it (i.e. -c copy).

Open a new issue, handling multiple things in a single report is hard and they tend to be forgotten.

@Spenhouet
Copy link

@Spenhouet Spenhouet commented Nov 4, 2016

A fix is still needed: #11116

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Nov 4, 2016

As the original reporter said:

Because ffmpeg cannot add metadata to the 3gp file

This applies to webm as well, which does not define "metadata" in the container officially. @jaimeMF mind a change?

@jaimeMF
Copy link
Collaborator

@jaimeMF jaimeMF commented Nov 4, 2016

@jaimeMF mind a change?

I personally don't mind, should we just change the order of the postprocessors or run it twice?

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Nov 5, 2016

should we just change the order of the postprocessors or run it twice?

I guess moving FFmpegMetadataPP to just before XAttrMetadataPP should work fine. By intuition, only the final file needs metadata, and if the final format does not support metadata, users have to change it.

@jaimeMF
Copy link
Collaborator

@jaimeMF jaimeMF commented Nov 5, 2016

I guess moving FFmpegMetadataPP to just before XAttrMetadataPP should work fine. By intuition, only the final file needs metadata, and if the final format does not support metadata, users have to change it.

It seems reasonable, it's fine for me.

@yan12125
Copy link
Collaborator

@yan12125 yan12125 commented Mar 6, 2017

Sorry for being late. This will be fixed in the next version.

@yan12125 yan12125 closed this in 54a3a88 Mar 6, 2017
yan12125 added a commit that referenced this issue Mar 11, 2017
The previous fix for #5594 is incorrect
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.