Replaced resample function by new implementation (2)

[AlexFuster]

#1392

[AlexFuster]

I don't get it, I run
python tools/lint/black_python.py --check tensorflow_io/core/python/ops/io_tensor_ops.py in my environment and works perfectly without error

[yongtang]

@AlexFuster if you have black installed, can you just run black audio_ops.py and see if the file is formatted automatically?

[AlexFuster]

done

[AlexFuster]

The tests seem to be taking too long. At least in macOS and linux

[yongtang]

@AlexFuster Can you rebase the PR with the latest master branch? We recently made some enhancement on testing which I think will resolve some issues shown in GitHub Actions CI.

[AlexFuster]

ok, here we go again

[AlexFuster]

Oh yeah, I forgot to mention It does just support floating point audios. Is this a problem?

[yongtang]

Oh yeah, I forgot to mention It does just support floating point audios. Is this a problem?

@AlexFuster The existing implementation support both int16 and float32. Though I think it will not be too difficult to add int16 support by casting to float32 and back.

[yongtang]

@AlexFuster The test failures are legitimate. The reason being

1. Tensor in tensorflow may not be treated as python native value (e.g, 1, 1.1, etc). Things like int(x) may not work when x is a tensor. Instead, an explicit op of tf.cast(x, tf.int32) is needed.
2. A tensor's shape may not be known statically before hand. So x.shape could just return None in case shape is not known yet.
3. When x is a tensor, the if...else... logic does not work as tensor's value is not statically available (before run). Instead a TF specific op of tf.case or tf.cond has to be used (you might notice the original implementation consists of tf.case to replace if...elif...else.

You may want to take a look at the test failures and see if some change is needed. I will also take a look.

[AlexFuster]

I don't think the error comes from any of those 3 problems. It is just tf.nn.conv1d not accepting int16 values as inputs. I'll try adding int16 support by casting and casting back as you said

[yongtang]

With int16 tests may pass, though in the original implementation the resample actually support passing rate_in, rate_out as tensor and input with unknown shape. I think with new implementation this ability is removed as I could see gcd and int(rate_in) will cause the failure if a tensor with unknown rank is passed for rate_in. The input may also cause some issue if it is a tensor with unknown shape. We can get the tests pass and address the rest in follow up PRs though.

[AlexFuster]

Why would you want a tensor with a shape other than () being passed as rate_in?

[yongtang]

Why would you want a tensor with a shape other than () being passed as rate_in?
@AlexFuster Sorry it should be a tensor without constant value. In graph mode (such as within tf.data.Dataset pipeline), user may pass input, rate_in, rate_out as tensor without const value. In that case, the graph mode will fail. They can be avoided by using ops that will work for tensor. For example, int(rate_in) can be converted to use tf.cast(rate_in, tf.int32), math.gcd can be replaced with ops as tf.experimental.numpy.gcd I think.

[google-cla]

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

[AlexFuster]

@googlebot I fixed it.

[AlexFuster]

ok, definitely the problem is in tf.nn.conv1d not accepting a tensor as a value for stride in graph mode (it does in eager mode). Any idea?