[video_player_avplay] Support parsing SMPE-EE subtitle attributes and image format subtitles. #947
Conversation
…efined subtitle styles
packages/video_player_avplay/lib/video_player_platform_interface.dart
Outdated
Show resolved
Hide resolved
|
@seungsoo47 Could you review this? |
OK |
| case SubtitleAttrType.subAttrWebvttCuePositionAlign: | ||
| case SubtitleAttrType.subAttrWebvttCueVertical: | ||
| case SubtitleAttrType.subAttrTimestamp: | ||
| case SubtitleAttrType.subAttrExtsubInde: |
There was a problem hiding this comment.
| case SubtitleAttrType.subAttrExtsubInde: | |
| case SubtitleAttrType.subAttrExtsubIndex: |
| return const Color(0x00000000); | ||
| } | ||
| String hexValue = colorValue.toRadixString(16); | ||
| if (hexValue.length < 6) { |
There was a problem hiding this comment.
How about changing 6 to a Constant?
| } | ||
| actualTextStyle = actualTextStyle.copyWith( | ||
| color: actualTextStyle.color! | ||
| .withValues(alpha: attr.attrValue as double)); |
There was a problem hiding this comment.
I think you need to check if the variable attr.attrValue is double.
And, the attrValue must have a value between 0.0 and 1.0.
Please ensure that the attrValue is within the correct range.
| if (hexValue.length == 6) { | ||
| hexValue = 'FF$hexValue'; | ||
| } | ||
| if (hexValue.length == 8) { |
There was a problem hiding this comment.
How about changing 8 to a Constant?
| switch (attr.attrType) { | ||
| // For text origin and extent. | ||
| case SubtitleAttrType.subAttrRegionXPos: | ||
| final double xPos = attr.attrValue as double; |
There was a problem hiding this comment.
I think you need to check if the variable attr.attrValue is double.
Please also check other items below.
There was a problem hiding this comment.
The type of attrValue corresponding to each SubtitleAttrType has been predefined in enum SubtitleAttrType{} and plus_player.cc->OnSubtitleData() , and the value of attrValue has already been processed in fromEventSubtitleAttrList(), so I think there is no need to perform type checking here.
| size, picture_width, picture_height); | ||
|
|
||
| int subtitle_mem_length = 0; | ||
| int channels = size / (picture_width * picture_height); |
There was a problem hiding this comment.
The picture_width and picture_height variables must not be 0.
How about adding the check code?
if (picture_width <= 0 || picture_height <= 0 || size <= 0) {
LOG_ERROR("[PlusPlayer] Invalid picture dimensions or size: w=%f, h=%f, size=%d",
picture_width, picture_height, size);
return;
}
And how about adding the overflow check code?
const double area = picture_width * picture_height;
if (area > static_cast<double>(std::numeric_limits<int>::max())) {
LOG_ERROR("[PlusPlayer] Picture area too large: %f", area);
return;
}
|
|
||
| PlusPlayer *self = reinterpret_cast<PlusPlayer *>(user_data); | ||
|
|
||
| int text_lines_count = 1; |
There was a problem hiding this comment.
Please change 1 to a Constant.
| float value_float; | ||
| std::memcpy(&value_float, &value_temp, sizeof(float)); | ||
| LOG_INFO("[PlusPlayer] Subtitle update: value<float>: %f", value_float); | ||
| const float *value_float = static_cast<const float *>(attr->value); |
There was a problem hiding this comment.
Please add type checking and null checking code.
4 participants
Main changes: