这是indexloc提供的服务，不要输入任何密码
Skip to content

Prevent MP4 export with invalid resolution #1833

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 29, 2024
Merged

Prevent MP4 export with invalid resolution #1833

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c9147ea
Prevent MP4 export with invalid resolution
J5lx Apr 26, 2024
113f92f
Fix Qt 5 build error
J5lx Apr 26, 2024
ce77921
Fix export dialog layout jumping
scribblemaniac Apr 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions app/src/exportmoviedialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,17 @@ ExportMovieDialog::ExportMovieDialog(QWidget *parent, Mode mode, FileType fileTy
} else {
setWindowTitle(tr("Export Movie"));
}

QSizePolicy policy = ui->unevenWidthLabel->sizePolicy();
policy.setRetainSizeWhenHidden(true);
ui->unevenWidthLabel->setSizePolicy(policy);
policy = ui->unevenHeightLabel->sizePolicy();
policy.setRetainSizeWhenHidden(true);
ui->unevenHeightLabel->setSizePolicy(policy);

connect(this, &ExportMovieDialog::filePathsChanged, this, &ExportMovieDialog::onFilePathsChanged);
connect(ui->widthSpinBox, static_cast<void(QSpinBox::*)(int)>(&QSpinBox::valueChanged), this, &ExportMovieDialog::validateResolution);
connect(ui->heightSpinBox, static_cast<void(QSpinBox::*)(int)>(&QSpinBox::valueChanged), this, &ExportMovieDialog::validateResolution);
}

ExportMovieDialog::~ExportMovieDialog()
Expand Down Expand Up @@ -66,6 +76,7 @@ void ExportMovieDialog::updateResolutionCombo( int index )

ui->widthSpinBox->setValue( camSize.width() );
ui->heightSpinBox->setValue( camSize.height() );
validateResolution();
}

void ExportMovieDialog::setDefaultRange(int startFrame, int endFrame, int endFrameWithSounds)
Expand Down Expand Up @@ -128,6 +139,7 @@ void ExportMovieDialog::onFilePathsChanged(QStringList filePaths)
ui->loopCheckBox->setChecked(false);
}
ui->transparencyCheckBox->setEnabled(supportsTransparency(filePath));
validateResolution();
}

bool ExportMovieDialog::supportsLooping(QString filePath) const
Expand All @@ -141,3 +153,13 @@ bool ExportMovieDialog::supportsTransparency(QString filePath) const
return filePath.endsWith(".apng", Qt::CaseInsensitive) ||
filePath.endsWith(".webm", Qt::CaseInsensitive);
}

void ExportMovieDialog::validateResolution()
{
const bool isMp4 = getFilePath().endsWith(".mp4", Qt::CaseInsensitive);
const bool widthValid = !isMp4 || ui->widthSpinBox->value() % 2 == 0;
const bool heightValid = !isMp4 || ui->heightSpinBox->value() % 2 == 0;
ui->unevenWidthLabel->setHidden(widthValid);
ui->unevenHeightLabel->setHidden(heightValid);
setOkButtonEnabled(widthValid && heightValid);
}
1 change: 1 addition & 0 deletions app/src/exportmoviedialog.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class ExportMovieDialog : public ImportExportDialog

bool supportsLooping(QString filePath) const;
bool supportsTransparency(QString filePath) const;
void validateResolution();

int mEndFrameWithSounds = 0;
int mEndFrame = 0;
Expand Down
5 changes: 5 additions & 0 deletions app/src/importexportdialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,11 @@ void ImportExportDialog::setInstructionsLabel(const QString& text)
ui->instructionsLabel->setText(text);
}

void ImportExportDialog::setOkButtonEnabled(const bool enabled)
{
ui->buttonBox->button(QDialogButtonBox::Ok)->setEnabled(enabled);
}

void ImportExportDialog::init()
{
switch (mMode)
Expand Down
1 change: 1 addition & 0 deletions app/src/importexportdialog.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class ImportExportDialog : public QDialog
void hideInstructionsLabel(bool hide);

void setInstructionsLabel(const QString& text);
void setOkButtonEnabled(bool enabled);

private slots:
void browse();
Expand Down
22 changes: 21 additions & 1 deletion app/ui/exportmovieoptions.ui
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
<property name="title">
<string>Resolution</string>
</property>
<layout class="QHBoxLayout" name="horizontalLayout">
<layout class="QHBoxLayout" name="horizontalLayout" stretch="1,0,1,1,0,1">
<item>
<widget class="QLabel" name="label">
<property name="sizePolicy">
Expand All @@ -60,6 +60,16 @@
</property>
</widget>
</item>
<item>
<widget class="QLabel" name="unevenWidthLabel">
<property name="toolTip">
<string>The MP4 format does not support odd width. Please specify an even width or use a different file format.</string>
Copy link
Member

@scribblemaniac scribblemaniac Apr 28, 2024

Choose a reason for hiding this comment

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

Technically MP4 is a container format and does support odd width/height, it is the specific codec we choose to use for exporting with the MP4 container that does not support this. While I don't think users need to know the technical details like this, I think maybe "MP4 exporter" instead of "MP4 format" may be better. It could avoid confusion in the future if we add more options to MP4 export, which could potentially allow videos with an odd width/height to work.

Copy link
Member Author

@J5lx J5lx Apr 28, 2024

Choose a reason for hiding this comment

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

As long as Pencil2D doesn’t allow the selection of alternative codecs, I don’t think the wording matters that much. We can worry about that when we do add such options. Until then, whatever you prefer is fine by me.

</property>
<property name="text">
<string notr="true">⚠</string>
</property>
</widget>
</item>
<item>
<widget class="QSpinBox" name="widthSpinBox">
<property name="maximum">
Expand Down Expand Up @@ -89,6 +99,16 @@
</property>
</widget>
</item>
<item>
<widget class="QLabel" name="unevenHeightLabel">
<property name="toolTip">
<string>The MP4 format does not support odd height. Please specify an even height or use a different file format.</string>
</property>
<property name="text">
<string notr="true">⚠</string>
</property>
</widget>
</item>
<item>
<widget class="QSpinBox" name="heightSpinBox">
<property name="maximum">
Expand Down