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

Update transcoder progress to fallback to framecount #480

Merged
merged 11 commits into from Aug 16, 2022

Conversation

jvl711
Copy link
Contributor

@jvl711 jvl711 commented Aug 13, 2022

In certain situations ffmpeg does not output a correct time in the status output. It seems like this happens most often when doing a remux of the file instead of a transcode. When the time is calculated as < 0 I am falling back to calculating the progress based on framecount.

I also made some minor changes in the gradle file to allow this project to build under netbeans.

CHANGELOG.md Outdated
@@ -1,5 +1,9 @@
# Change Log

## Next
* Updated grable script so that project could build in Netbeans
Copy link
Member

Choose a reason for hiding this comment

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

s/grable/gradle

build.gradle Outdated
@@ -69,7 +69,7 @@ sourceCompatibility = 1.7

compileJava {
options.warnings = false
options.compilerArgs.addAll(['--release', '8'])
//options.compilerArgs.addAll(['--release', '8'])
Copy link
Member

Choose a reason for hiding this comment

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

Why did you remove the Java 8 compatibility?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am going to put this back. I took it out because I was not able to build at all with it in. I am getting this error with it in "invalid flag: --release". I am wondering if adding this is necessary if we are doing targetCompatibility = 1.7 and sourceCompatibility = 1.7.

build.gradle Outdated Show resolved Hide resolved
@@ -120,6 +120,7 @@ public float getPercentComplete()
if (tempy != null)
{
long fullLength = clipDuration == 0 ? (mf.getDuration(transcodeSegment) - clipStartTime) : clipDuration;

Copy link
Member

Choose a reason for hiding this comment

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

Can you undo this line removal?

}

int frame = Integer.parseInt(frameStr);
float fps = FFMPEGTranscoder.this.sourceFormat.getVideoFormat().getFps();
Copy link
Member

Choose a reason for hiding this comment

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

Are the frame counts it's outputting here relative to the source frame rate or the destination frame rate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did some testing since the FFmpeg documentation was not specific. I am fairly certain it is the output format that dictates the frame count. It does not look like we have a container format for what format we are going to. Do you have a recommendation for getting the target fps?

Copy link
Member

Choose a reason for hiding this comment

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

Aside from tracking what we launch...not really. From what I can tell...the only impact this change will have is on the progress duration showed in the UI, so probably not worth more effort. Is there anything else you noticed it would actually fix?

lastXcodeStreamTime = Math.round(1000 * Double.parseDouble(timeStr));
double time = Double.parseDouble(timeStr);

//Fallback to using frame count to determin time if the time is < 0
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean "time is < 0" here, because on the next line you're doing "time <= 1" for the fallback case?

build.gradle Outdated
def getBuildNumber() {
def buildVerText = new File('java/sage/SageConstants.java').text
def getBuildNumber()
{
Copy link
Member

Choose a reason for hiding this comment

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

Put this brace back on the prior line (it's an unnecessary change).

build.gradle Outdated
def buildVerText = new File('java/sage/SageConstants.java').text
def getBuildNumber()
{
def buildVer
Copy link
Member

Choose a reason for hiding this comment

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

Erase this line and the next line and put the 'def' right before buildVer a coouple lines down.

build.gradle Outdated
@@ -355,10 +358,14 @@ def getBuildNumber() {
*/
task(updateBuildNumber) {
doLast {

Copy link
Member

Choose a reason for hiding this comment

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

Remove these extraneous blank lines that were added.

@@ -22,5 +22,5 @@ private SageConstants()
// Non-instantiable
}

public static final int BUILD_VERSION = 441;
public static final int BUILD_VERSION = 999;
Copy link
Member

Choose a reason for hiding this comment

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

Don't modify this file.

}

int frame = Integer.parseInt(frameStr);
float fps = FFMPEGTranscoder.this.sourceFormat.getVideoFormat().getFps();
Copy link
Member

Choose a reason for hiding this comment

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

Aside from tracking what we launch...not really. From what I can tell...the only impact this change will have is on the progress duration showed in the UI, so probably not worth more effort. Is there anything else you noticed it would actually fix?

@Narflex Narflex merged commit d78189c into google:master Aug 16, 2022
@Narflex
Copy link
Member

Narflex commented Aug 16, 2022

Thanks for the update!

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.

None yet

2 participants