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

Check all streams to determine type #41

Closed
wants to merge 2 commits into from
Closed

Conversation

m0vse
Copy link

@m0vse m0vse commented Mar 4, 2019

Look for video content in all detected streams not just the first.

Look for video content in all detected streams not just the first.
@dotarmin
Copy link
Contributor

dotarmin commented Mar 5, 2019

@m0vse, thanks for this PR! I will make sure somebody checks is asap!

@jnx, can you check this and try it?

@dotarmin dotarmin requested a review from jnx March 5, 2019 09:56
@dotarmin
Copy link
Contributor

dotarmin commented Mar 6, 2019

ping @jnx

src/scanner.js Outdated
tb = [ fr[1], fr[0] ]
}
break
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be a for of loops since streams is an array?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for (const { pix_fmt } of json.streams) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem with this is that it will set audio files with a cover as image instead of audio... so we also need to check the disposition of the stream

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes a good point, how about if we also check to see if there is a codec_type:audio element in one of the streams? This would have the same issue if the image stream is before the audio one (not sure if that can happen) but I would say that is a far less likely occurrence to audio being before video which seems to happen in around 30% of videos that I receive!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have spotted the deliberate mistake in my previous comment... We would also need some way to determine which is the "primary" stream I guess?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry, but isn't it more logical and practical than if there is video to switch to the video stream and take the data from it? I have problems when Vegas exports on Mainconcept mp4, stream 1 Audio, and 2 Video.

Use for of loop instead of in.
@dotarmin
Copy link
Contributor

dotarmin commented Mar 13, 2019

Any updates on this PR?

@dotarmin dotarmin changed the title Update scanner.js Look for content in all detected streams Mar 26, 2019
@dotarmin dotarmin changed the title Look for content in all detected streams Check all streams to determine type Apr 25, 2019
@dotarmin dotarmin self-assigned this Jun 6, 2019
@grahamspr86
Copy link

grahamspr86 commented Nov 19, 2019

Any updates on this PR?

Have tested this and appears to correctly identify some videos that I had coming though as audio

jamesremuscat added a commit to jamesremuscat/media-scanner that referenced this pull request Feb 27, 2023
Refs CasparCG#41 but takes a
different, possibly differently wrong, approach...
@Julusian
Copy link
Member

Julusian commented Sep 4, 2023

Rebased and merged as 41dbfdd

@Julusian Julusian closed this Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants